[RFC PATCH v3 00/10] Rewrite tests into new API + fixes

[RFC PATCH v3 00/10] Rewrite tests into new API + fixes

[Petr Vorel]

Hi,

changes v2->v3:
* Fixed some of errors caused by test order.

* ima_boot_aggregate
  - max event size is now 1MB according to spec

* ima_mmap
  - reduce sleep + log it
  - rewritten into new API

* ima_measurements.sh
  - don't require iversion for kernel >= 4.16
  - avoid using tmpfs

* ima_policy.sh
  - improved detection of policy writability
  - merge test2 and test3

* ima_violations.sh
  - avoid using tmpfs
  - improved grepping logs (no sleep is needed)

* ima_tpm.sh
  - Improve error messages

TODO:
* fix problems with violations tests (see patch 02/10).
* detect whether policy must be signed (currently tests assume the
policy does not need to be signed):
https://lists.linux.it/pipermail/ltp/2018-April/007702.html
http://lists.linux.it/pipermail/ltp/2018-January/006970.html

Comments and patches are welcome.

Kind regards,
Petr

Petr Vorel (10):
  security/ima: Rewrite tests into new API + fixes
  security/ima: Change order of tests
  ima/ima_policy.sh: Improve check of policy writability
  ima/ima_policy.sh: Load whole policy with cat
  ima/ima_boot_aggregate: Increase MAX_EVENT_SIZE to 1MB
  ima/tpm.sh: Use evmctl + other fixes
  ima/ima_mmap: Reduce sleep + log it
  ima/{ima_measurements,ima_violations}.sh: Avoid running on tmpfs
  ima: CRYPTO_LIBS are needed only for ima_boot_aggregate
  ima/ima_mmap: Rewrite to new library

 runtest/ima                                        |   8 +-
 testcases/kernel/security/integrity/.gitignore     |   1 -
 .../kernel/security/integrity/ima/src/Makefile     |   2 +-
 .../integrity/ima/src/ima_boot_aggregate.c         |  16 +-
 .../security/integrity/ima/src/ima_measure.c       | 219 ------------------
 .../kernel/security/integrity/ima/src/ima_mmap.c   |  82 +++----
 .../integrity/ima/tests/ima_measurements.sh        | 247 +++++++++++----------
 .../security/integrity/ima/tests/ima_policy.sh     | 169 ++++++--------
 .../security/integrity/ima/tests/ima_setup.sh      | 141 ++++++------
 .../kernel/security/integrity/ima/tests/ima_tpm.sh | 165 ++++++--------
 .../security/integrity/ima/tests/ima_violations.sh | 228 ++++++++++---------
 11 files changed, 530 insertions(+), 748 deletions(-)
 delete mode 100644 testcases/kernel/security/integrity/ima/src/ima_measure.c
 mode change 100755 => 100644 testcases/kernel/security/integrity/ima/tests/ima_setup.sh

-- 
2.16.3

[RFC PATCH v3 01/10] security/ima: Rewrite tests into new API + fixes

[Petr Vorel]

* simplify code, remove duplicity

* ima_measurements.sh:
  - add support for "ima-ng" and "ima-sig" IMA measurement templates
  - add support for most of hash algorithms is defined in
    include/uapi/linux/hash_info.h (kernel headers); algorithms are
    detected from last occurrence of tested file in
    /sys/kernel/security/ima/ascii_runtime_measurements
  - Improve iversion check:
    * check i_version mount option only for ext[2-4] filesystems (other
      filesystems don't report it), TCONF when not mounted with it
    * XFS has iversion support from >= V5, TCONF when older version
    * previous 2 checks are only for kernel < 4.16 (kernel with commit
    ac0bf025d2c0 "ima: Use i_version only when filesystem supports it"
    files on filesystems, which do not support i_version, will now *always*
    be re-measured, i_version is in this case only a performance improvement)
  - chown only UID (GID of nobody is different on some OS, so it's
    better not to set it as it's not necessary for the test)

* ima_policy.sh:
  - break tests instead of print TINFO when kernel is not configured to
    enable multiple writes to the IMA policy (IMA_WRITE_POLICY)
  - add warning when policy has been updated that reboot is needed

* ima_violations.sh:
  - change check to measure occurrence of messages in log (previous way
    to grep tail of the log was buggy)
  - verification: add 5 attempts to check log before fail

* ima_tpm.sh
  - change TCONF to TINFO in test1 (code behind that was never run)
  - make variables local

* runtest file
  - rename the test ids to match the shell script names (more descriptive)
    and remove duplicate whitespace
  - change TCONF to TINFO in test2 when TPM not enabled

Thanks a lot to Mimi Zohar for patient review and tips.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 runtest/ima                                        |   8 +-
 .../integrity/ima/tests/ima_measurements.sh        | 246 +++++++++++----------
 .../security/integrity/ima/tests/ima_policy.sh     | 153 ++++++-------
 .../security/integrity/ima/tests/ima_setup.sh      | 113 ++++------
 .../kernel/security/integrity/ima/tests/ima_tpm.sh | 142 +++++-------
 .../security/integrity/ima/tests/ima_violations.sh | 224 +++++++++----------
 6 files changed, 423 insertions(+), 463 deletions(-)
 mode change 100755 => 100644 testcases/kernel/security/integrity/ima/tests/ima_setup.sh

diff --git a/runtest/ima b/runtest/ima
index 251458af4..bcae16bb7 100644
--- a/runtest/ima
+++ b/runtest/ima
@@ -1,5 +1,5 @@
 #DESCRIPTION:Integrity Measurement Architecture (IMA)
-ima01   ima_measurements.sh
-ima02   ima_policy.sh
-ima03   ima_tpm.sh
-ima04   ima_violations.sh
+ima_measurements ima_measurements.sh
+ima_policy ima_policy.sh
+ima_tpm ima_tpm.sh
+ima_violations ima_violations.sh
diff --git a/testcases/kernel/security/integrity/ima/tests/ima_measurements.sh b/testcases/kernel/security/integrity/ima/tests/ima_measurements.sh
index a3c357c8b..0bceeb71f 100755
--- a/testcases/kernel/security/integrity/ima/tests/ima_measurements.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_measurements.sh
@@ -1,139 +1,161 @@
 #!/bin/sh
-
-################################################################################
-##                                                                            ##
-## Copyright (C) 2009 IBM Corporation                                         ##
-##                                                                            ##
-## This program is free software;  you can redistribute it and#or modify      ##
-## it under the terms of the GNU General Public License as published by       ##
-## the Free Software Foundation; either version 2 of the License, or          ##
-## (at your option) any later version.                                        ##
-##                                                                            ##
-## This program is distributed in the hope that it will be useful, but        ##
-## WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY ##
-## or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License   ##
-## for more details.                                                          ##
-##                                                                            ##
-## You should have received a copy of the GNU General Public License          ##
-## along with this program;  if not, write to the Free Software Foundation,   ##
-## Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA           ##
-##                                                                            ##
-################################################################################
+# Copyright (c) 2009 IBM Corporation
+# Copyright (c) 2018 Petr Vorel <pvorel@suse.cz>
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation; either version 2 of
+# the License, or (at your option) any later version.
 #
-# File :        ima_measurements.sh
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
 #
-# Description:  This file verifies measurements are added to the measurement
-# 		list based on policy.
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
 #
-# Author:       Mimi Zohar, zohar@ibm.vnet.ibm.com
-################################################################################
-export TST_TOTAL=3
-export TCID="ima_measurements"
+# Author: Mimi Zohar, zohar@ibm.vnet.ibm.com
+#
+# Verify that measurements are added to the measurement list based on policy.
+
+TST_NEEDS_CMDS="awk"
+TST_SETUP="setup"
+TST_CNT=3
 
-init()
+. ima_setup.sh
+
+setup()
 {
-	tst_check_cmds sha1sum
+	DEFAULT_DIGEST_OLD_FORMAT="sha1"
+	TEST_FILE="$PWD/test.txt"
 
-	# verify using default policy
-	if [ ! -f "$IMA_DIR/policy" ]; then
-		tst_resm TINFO "not using default policy"
-	fi
+	POLICY="$IMA_DIR/policy"
+	[ -f "$POLICY" ] || tst_res TINFO "not using default policy"
+
+	DIGEST_INDEX=
+	grep -q "ima-ng" $ASCII_MEASUREMENTS && DIGEST_INDEX=1
+	grep -q "ima-sig" $ASCII_MEASUREMENTS && DIGEST_INDEX=2
 }
 
-# Function:     test01
-# Description   - Verify reading a file causes a new measurement to
-#		  be added to the IMA measurement list.
-test01()
+# TODO: find support for rmd128 rmd256 rmd320 wp256 wp384 tgr128 tgr160
+compute_hash()
 {
-	# Create file test.txt
-	cat > test.txt <<-EOF
-	$(date) - this is a test file
-	EOF
-	if [ $? -ne 0 ]; then
-		tst_brkm TBROK "Unable to create test file"
-	fi
+	local digest="$1"
+	local file="$2"
 
-	# Calculating the sha1sum of test.txt should add
-	# the measurement to the measurement list.
-	# (Assumes SHA1 IMA measurements.)
-	hash=$(sha1sum "test.txt" | sed 's/  -//')
-
-	# Check if the file is measured
-	# (i.e. contained in the ascii measurement list.)
-	cat /sys/kernel/security/ima/ascii_runtime_measurements > measurements
-	sleep 1
-	$(grep $hash measurements > /dev/null)
-	if [ $? -ne 0 ]; then
-		tst_resm TFAIL "TPM ascii measurement list does not contain sha1sum"
-	else
-		tst_resm TPASS "TPM ascii measurement list contains sha1sum"
-	fi
+	hash="$(${digest}sum $file 2>/dev/null | cut -f1 -d ' ')"
+	[ -n "$hash" ] && { echo $hash; return; }
+
+	hash="$(openssl $digest $file 2>/dev/null | cut -f2 -d ' ')"
+	[ -n "$hash" ] && { echo $hash; return; }
+
+	# uncommon ciphers
+	local arg="$digest"
+	case "$digest" in
+	tgr192) arg="tiger" ;;
+	wp512) arg="whirlpool" ;;
+	esac
+
+	hash="$(rhash --$arg $file 2>/dev/null | cut -f1 -d ' ')"
+	[ -n "$hash" ] && { echo $hash; return; }
 }
 
-# Function:     test02
-# Description	- Verify modifying, then reading, a file causes a new
-# 		  measurement to be added to the IMA measurement list.
-test02()
+ima_check()
 {
-	# Modify test.txt
-	echo $(date) - file modified >> test.txt
+	local digest="$DEFAULT_DIGEST_OLD_FORMAT"
+	local hash expected_hash line
+
+	# need to read file to get updated $ASCII_MEASUREMENTS
+	cat $TEST_FILE > /dev/null
+
+	line="$(grep $TEST_FILE $ASCII_MEASUREMENTS | tail -1)"
+	[ -n "$line" ] || tst_res TFAIL "cannot find measurement for '$TEST_FILE'"
 
-	# Calculating the sha1sum of test.txt should add
-	# the new measurement to the measurement list
-	hash=$(sha1sum test.txt | sed 's/  -//')
+	[ "$DIGEST_INDEX" ] && digest="$(echo "$line" | awk '{print $(NF-'$DIGEST_INDEX')}' | cut -d ':' -f 1)"
+	hash="$(echo "$line" | awk '{print $(NF-1)}' | cut -d ':' -f 2)"
 
-	# Check if the new measurement exists
-	cat /sys/kernel/security/ima/ascii_runtime_measurements > measurements
-	$(grep $hash measurements > /dev/null)
+	tst_res TINFO "computing hash for $digest digest"
+	expected_hash="$(compute_hash $digest $TEST_FILE)" || \
+		{ tst_res TCONF "cannot compute hash for '$digest' digest"; return; }
 
-	if [ $? -ne 0 ]; then
-		tst_resm TFAIL "Modified file not measured"
-		tst_resm TINFO "iversion not supported; or not mounted with iversion"
+	if [ "$hash" = "$expected_hash" ]; then
+		tst_res TPASS "correct hash found"
 	else
-		tst_resm TPASS "Modified file measured"
+		tst_res TFAIL "hash not found"
 	fi
 }
 
-# Function:     test03
-# Description 	- Verify files are measured based on policy
-#		(Default policy does not measure user files.)
-test03()
+check_iversion_support()
 {
-	# create file user-test.txt
-	mkdir -m 0700 user
-	chown nobody.nobody user
-	cd user
-	hash=0
-
-	# As user nobody, create and cat the new file
-	# (The LTP tests assumes existence of 'nobody'.)
-	sudo -n -u nobody sh -c "echo $(date) - create test.txt > ./test.txt;
-				 cat ./test.txt > /dev/null"
-
-	# Calculating the hash will add the measurement to the measurement
-	# list, so only calc the hash value after getting the measurement
-	# list.
-	cat /sys/kernel/security/ima/ascii_runtime_measurements > measurements
-	hash=$(sha1sum test.txt | sed 's/  -//')
-	cd - >/dev/null
-
-	# Check if the file is measured
-	grep $hash measurements > /dev/null
-	if [ $? -ne 0 ]; then
-		tst_resm TPASS "user file test.txt not measured"
-	else
-		tst_resm TFAIL "user file test.txt measured"
-	fi
+	local device mount fs
+
+	tst_kvcmp -ge "4.16" && return 0
+
+	device="$(df . | sed -e 1d | cut -f1 -d ' ')"
+	mount="$(grep $device /proc/mounts | head -1)"
+	fs="$(echo $mount | awk '{print $3'})"
+
+	case "$fs" in
+	ext[2-4])
+		if ! echo "$mount" | grep -q -w "i_version"; then
+			tst_res TCONF "device '$device' is not mounted with iversion, please mount it with 'mount $device -o remount,iversion'"
+			return 1
+		fi
+		;;
+	xfs)
+		if dmesg | grep -q "XFS.*Mounting V[1-4] Filesystem"; then
+			tst_res TCONF "XFS Filesystem >= V5 required for iversion support"
+			return 1
+		fi
+		;;
+	'')
+		tst_res TWARN "could not find mount info for device '$device'"
+		;;
+	esac
+
+	return 0
 }
 
-. ima_setup.sh
+test1()
+{
+	tst_res TINFO "verify adding record to the IMA measurement list"
+	ROD echo "$(date) this is a test file" \> $TEST_FILE
+	ima_check
+}
+
+test2()
+{
 
-setup
-TST_CLEANUP=cleanup
+	tst_res TINFO "verify updating record in the IMA measurement list"
+	check_iversion_support || return
+	ROD echo "$(date) modified file" \> $TEST_FILE
+	ima_check
+}
 
-init
-test01
-test02
-test03
+test3()
+{
+	local user="nobody"
+	local dir="$PWD/user"
+	local file="$dir/test.txt"
+
+	# Default policy does not measure user files
+	tst_res TINFO "verify not measuring user files"
+	tst_check_cmds sudo
+
+	if ! id $user >/dev/null 2>/dev/null; then
+		tst_res TCONF "missing system user $user (wrong installation)"
+		return
+	fi
+
+	mkdir -m 0700 $dir
+	chown $user $dir
+	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"
+	cd ..
+
+	EXPECT_FAIL "grep $file $ASCII_MEASUREMENTS"
+}
 
-tst_exit
+tst_run
diff --git a/testcases/kernel/security/integrity/ima/tests/ima_policy.sh b/testcases/kernel/security/integrity/ima/tests/ima_policy.sh
index ad5900975..2efa90038 100755
--- a/testcases/kernel/security/integrity/ima/tests/ima_policy.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_policy.sh
@@ -1,127 +1,114 @@
 #!/bin/sh
-################################################################################
-##                                                                            ##
-## Copyright (C) 2009 IBM Corporation                                         ##
-##                                                                            ##
-## This program is free software;  you can redistribute it and#or modify      ##
-## it under the terms of the GNU General Public License as published by       ##
-## the Free Software Foundation; either version 2 of the License, or          ##
-## (at your option) any later version.                                        ##
-##                                                                            ##
-## This program is distributed in the hope that it will be useful, but        ##
-## WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY ##
-## or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License   ##
-## for more details.                                                          ##
-##                                                                            ##
-## You should have received a copy of the GNU General Public License          ##
-## along with this program;  if not, write to the Free Software               ##
-## Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA    ##
-##                                                                            ##
-################################################################################
+# Copyright (c) 2009 IBM Corporation
+# Copyright (c) 2018 Petr Vorel <pvorel@suse.cz>
 #
-# File :        ima_policy.sh
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation; either version 2 of
+# the License, or (at your option) any later version.
 #
-# Description:  This file tests replacing the default integrity measurement
-#		policy.
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
 #
-# Author:       Mimi Zohar, zohar@ibm.vnet.ibm.com
-################################################################################
-export TST_TOTAL=3
-export TCID="ima_policy"
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+#
+# Author: Mimi Zohar, zohar@ibm.vnet.ibm.com
+#
+# Test replacing the default integrity measurement policy.
+
+TST_SETUP="setup"
+TST_CNT=3
 
-init()
+. ima_setup.sh
+
+setup()
 {
-	# verify using default policy
-	IMA_POLICY=$IMA_DIR/policy
-	if [ ! -f $IMA_POLICY ]; then
-		tst_resm TINFO "default policy already replaced"
-	fi
+	IMA_POLICY="$IMA_DIR/policy"
+	[ -f $IMA_POLICY ] || \
+		tst_brk TCONF "IMA policy already loaded and kernel not configured to enable multiple writes it"
 
-	VALID_POLICY=$LTPROOT/testcases/data/ima_policy/measure.policy
-	if [ ! -f $VALID_POLICY ]; then
-		tst_resm TINFO "missing $VALID_POLICY"
-	fi
+	VALID_POLICY="$TST_DATAROOT/measure.policy"
+	[ -f $VALID_POLICY ] || tst_brk TCONF "missing $VALID_POLICY"
 
-	INVALID_POLICY=$LTPROOT/testcases/data/ima_policy/measure.policy-invalid
-	if [ ! -f $INVALID_POLICY ]; then
-		tst_resm TINFO "missing $INVALID_POLICY"
-	fi
+	INVALID_POLICY="$TST_DATAROOT/measure.policy-invalid"
+	[ -f $INVALID_POLICY ] || tst_brk TCONF "missing $INVALID_POLICY"
 }
 
 load_policy()
 {
+	local ret
+
 	exec 2>/dev/null 4>$IMA_POLICY
-	if [ $? -ne 0 ]; then
-		exit 1
-	fi
+	[ $? -eq 0 ] || exit 1
 
 	cat $1 |
-	while read line ; do
-	{
-		if [ "${line#\#}" = "${line}" ] ; then
-			echo $line >&4 2> /dev/null
+	while read line; do
+		if [ "${line#\#}" = "${line}" ]; then
+			echo "$line" >&4 2> /dev/null
 			if [ $? -ne 0 ]; then
 				exec 4>&-
 				return 1
 			fi
 		fi
-	}
 	done
-}
+	ret=$?
 
+	[ $ret -eq 0 ] && \
+		tst_res TINFO "IMA policy updated, please reboot after testing to restore settings"
 
-# Function:     test01
-# Description   - Verify invalid policy doesn't replace default policy.
-test01()
+	return $ret
+}
+
+test1()
 {
+	tst_res TINFO "verify that invalid policy isn't loaded"
+
+	local p1
+
 	load_policy $INVALID_POLICY & p1=$!
 	wait "$p1"
 	if [ $? -ne 0 ]; then
-		tst_resm TPASS "didn't load invalid policy"
+		tst_res TPASS "didn't load invalid policy"
 	else
-		tst_resm TFAIL "loaded invalid policy"
+		tst_res TFAIL "loaded invalid policy"
 	fi
 }
 
-# Function:     test02
-# Description	- Verify policy file is opened sequentially, not concurrently
-#		  and install new policy
-test02()
+test2()
 {
-	load_policy $VALID_POLICY & p1=$!  # forked process 1
-	load_policy $VALID_POLICY & p2=$!  # forked process 2
-	wait "$p1"; RC1=$?
-	wait "$p2"; RC2=$?
-	if [ $RC1 -eq 0 ] && [ $RC2 -eq 0 ]; then
-		tst_resm TFAIL "measurement policy opened concurrently"
-	elif [ $RC1 -eq 0 ] || [ $RC2 -eq 0 ]; then
-		tst_resm TPASS "replaced default measurement policy"
+	tst_res TINFO "verify that policy file is not opened concurrently"
+
+	local p1 p2 rc1 rc2
+
+	load_policy $VALID_POLICY & p1=$!
+	load_policy $VALID_POLICY & p2=$!
+	wait "$p1"; rc1=$?
+	wait "$p2"; rc2=$?
+	if [ $rc1 -eq 0 ] && [ $rc2 -eq 0 ]; then
+		tst_res TFAIL "policy opened concurrently"
+	elif [ $rc1 -eq 0 ] || [ $rc2 -eq 0 ]; then
+		tst_res TPASS "policy was loaded just by one process"
 	else
-		tst_resm TFAIL "problems opening measurement policy"
+		tst_res TFAIL "problem loading policy"
 	fi
 }
 
-# Function:     test03
-# Description 	- Verify can't load another measurement policy.
-test03()
+test3()
 {
+	tst_res TINFO "verify that invalid policy isn't loaded"
+
+	local p1
+
 	load_policy $INVALID_POLICY & p1=$!
 	wait "$p1"
 	if [ $? -ne 0 ]; then
-		tst_resm TPASS "didn't replace valid policy"
+		tst_res TPASS "didn't replace valid policy"
 	else
-		tst_resm TFAIL "replaced valid policy"
+		tst_res TFAIL "replaced valid policy"
 	fi
 }
 
-. ima_setup.sh
-
-setup
-TST_CLEANUP=cleanup
-
-init
-test01
-test02
-test03
-
-tst_exit
+tst_run
diff --git a/testcases/kernel/security/integrity/ima/tests/ima_setup.sh b/testcases/kernel/security/integrity/ima/tests/ima_setup.sh
old mode 100755
new mode 100644
index 0ff38d23b..c08e2579e
--- a/testcases/kernel/security/integrity/ima/tests/ima_setup.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_setup.sh
@@ -1,86 +1,69 @@
 #!/bin/sh
-################################################################################
-##                                                                            ##
-## Copyright (C) 2009 IBM Corporation                                         ##
-##                                                                            ##
-## This program is free software;  you can redistribute it and#or modify      ##
-## it under the terms of the GNU General Public License as published by       ##
-## the Free Software Foundation; either version 2 of the License, or          ##
-## (at your option) any later version.                                        ##
-##                                                                            ##
-## This program is distributed in the hope that it will be useful, but        ##
-## WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY ##
-## or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License   ##
-## for more details.                                                          ##
-##                                                                            ##
-## You should have received a copy of the GNU General Public License          ##
-## along with this program;  if not, write to the Free Software Foundation,   ##
-## Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA           ##
-##                                                                            ##
-################################################################################
+# Copyright (c) 2009 IBM Corporation
+# Copyright (c) 2018 Petr Vorel <pvorel@suse.cz>
 #
-# File :        ima_setup.sh
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation; either version 2 of
+# the License, or (at your option) any later version.
 #
-# Description:  setup/cleanup routines for the integrity tests.
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
 #
-# Author:       Mimi Zohar, zohar@ibm.vnet.ibm.com
-################################################################################
-. test.sh
-mount_sysfs()
-{
-	SYSFS=$(mount 2>/dev/null | awk '$5 == "sysfs" { print $3 }')
-	if [ "x$SYSFS" = x ] ; then
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+#
+# Author: Mimi Zohar, zohar@ibm.vnet.ibm.com
 
-		SYSFS=/sys
+TST_TESTFUNC="test"
+TST_SETUP_CALLER="$TST_SETUP"
+TST_SETUP="ima_setup"
+TST_CLEANUP="ima_cleanup"
+TST_NEEDS_TMPDIR=1
+TST_NEEDS_ROOT=1
 
-		test -d $SYSFS || mkdir -p $SYSFS 2>/dev/null
-		if [ $? -ne 0 ] ; then
-			tst_brkm TBROK "Failed to mkdir $SYSFS"
-		fi
-		if ! mount -t sysfs sysfs $SYSFS 2>/dev/null ; then
-			tst_brkm TBROK "Failed to mount $SYSFS"
-		fi
+. tst_test.sh
 
-	fi
-}
+SYSFS="/sys"
+UMOUNT=
 
-mount_securityfs()
+mount_helper()
 {
-	SECURITYFS=$(mount 2>/dev/null | awk '$5 == "securityfs" { print $3 }')
-	if [ "x$SECURITYFS" = x ] ; then
-
-		SECURITYFS="$SYSFS/kernel/security"
+	local type="$1"
+	local default_dir="$2"
+	local dir
 
-		test -d $SECURITYFS || mkdir -p $SECURITYFS 2>/dev/null
-		if [ $? -ne 0 ] ; then
-			tst_brkm TBROK "Failed to mkdir $SECURITYFS"
-		fi
-		if ! mount -t securityfs securityfs $SECURITYFS 2>/dev/null ; then
-			tst_brkm TBROK "Failed to mount $SECURITYFS"
-		fi
+	dir="$(grep ^$type /proc/mounts | cut -d ' ' -f2 | head -1)"
+	[ -n "$dir" ] && { echo "$dir"; return; }
 
+	if ! mkdir -p $default_dir; then
+		tst_brk TBROK "Failed to create $default_dir"
 	fi
+	if ! mount -t $type $type $default_dir; then
+		tst_brk TBROK "Failed to mount $type"
+	fi
+	UMOUNT="$default_dir $UMOUNT"
+	echo $default_dir
 }
 
-setup()
+ima_setup()
 {
-	tst_require_root
-
-	tst_tmpdir
+	SECURITYFS="$(mount_helper securityfs $SYSFS/kernel/security)"
 
-	mount_sysfs
+	IMA_DIR="$SECURITYFS/ima"
+	[ -d "$IMA_DIR" ] || tst_brk TCONF "IMA not enabled in kernel"
+	ASCII_MEASUREMENTS="$IMA_DIR/ascii_runtime_measurements"
+	BINARY_MEASUREMENTS="$IMA_DIR/binary_runtime_measurements"
 
-	# mount securityfs if it is not already mounted
-	mount_securityfs
-
-	# IMA must be configured in the kernel
-	IMA_DIR=$SECURITYFS/ima
-	if [ ! -d "$IMA_DIR" ]; then
-		tst_brkm TCONF "IMA not enabled in kernel"
-	fi
+	[ -n "$TST_SETUP_CALLER" ] && $TST_SETUP_CALLER
 }
 
-cleanup()
+ima_cleanup()
 {
-	tst_rmdir
+	local dir
+	for dir in $UMOUNT; do
+		umount $dir
+	done
 }
diff --git a/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh b/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
index 333bf5f8a..ed45ab8d2 100755
--- a/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
@@ -1,70 +1,57 @@
 #!/bin/sh
-
-################################################################################
-##                                                                            ##
-## Copyright (C) 2009 IBM Corporation                                         ##
-##                                                                            ##
-## This program is free software;  you can redistribute it and#or modify      ##
-## it under the terms of the GNU General Public License as published by       ##
-## the Free Software Foundation; either version 2 of the License, or          ##
-## (at your option) any later version.                                        ##
-##                                                                            ##
-## This program is distributed in the hope that it will be useful, but        ##
-## WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY ##
-## or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License   ##
-## for more details.                                                          ##
-##                                                                            ##
-## You should have received a copy of the GNU General Public License          ##
-## along with this program;  if not, write to the Free Software               ##
-## Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA    ##
-##                                                                            ##
-################################################################################
+# Copyright (c) 2009 IBM Corporation
+# Copyright (c) 2018 Petr Vorel <pvorel@suse.cz>
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation; either version 2 of
+# the License, or (at your option) any later version.
 #
-# File :        ima_tpm.sh
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
 #
-# Description:  This file verifies the boot and PCR aggregates
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
 #
-# Author:       Mimi Zohar, zohar@ibm.vnet.ibm.com
+# Author: Mimi Zohar, zohar@ibm.vnet.ibm.com
 #
-# Return        - zero on success
-#               - non zero on failure. return value from commands ($RC)
-################################################################################
-export TST_TOTAL=3
-export TCID="ima_tpm"
+# Verify the boot and PCR aggregates.
 
-init()
-{
-	tst_check_cmds ima_boot_aggregate ima_measure
-}
+TST_NEEDS_CMDS="ima_boot_aggregate ima_measure"
+TST_CNT=3
+
+. ima_setup.sh
 
-# Function:     test01
-# Description   - Verify boot aggregate value is correct
-test01()
+test1()
 {
-	zero="0000000000000000000000000000000000000000"
+	tst_res TINFO "verify boot aggregate"
+
+	local zero="0000000000000000000000000000000000000000"
+	local tpm_bios="$SECURITYFS/tpm0/binary_bios_measurements"
+	local ima_measurements="$ASCII_MEASUREMENTS"
+	local boot_aggregate boot_hash ima_hash line
 
 	# IMA boot aggregate
-	ima_measurements=$SECURITYFS/ima/ascii_runtime_measurements
 	read line < $ima_measurements
-	ima_aggr=$(expr substr "${line}" 49 40)
+	ima_hash=$(expr substr "${line}" 49 40)
 
-	# verify TPM is available and enabled.
-	tpm_bios=$SECURITYFS/tpm0/binary_bios_measurements
 	if [ ! -f "$tpm_bios" ]; then
-		tst_brkm TCONF "TPM not builtin kernel, or TPM not enabled"
+		tst_res TINFO "TPM not builtin kernel, or TPM not enabled"
 
-		if [ "${ima_aggr}" = "${zero}" ]; then
-			tst_resm TPASS "bios boot aggregate is 0."
+		if [ "${ima_hash}" = "${zero}" ]; then
+			tst_res TPASS "bios boot aggregate is 0"
 		else
-			tst_resm TFAIL "bios boot aggregate is not 0."
+			tst_res TFAIL "bios boot aggregate is not 0"
 		fi
 	else
 		boot_aggregate=$(ima_boot_aggregate $tpm_bios)
-		boot_aggr=$(expr substr $boot_aggregate 16 40)
-		if [ "x${ima_aggr}" = "x${boot_aggr}" ]; then
-			tst_resm TPASS "bios aggregate matches IMA boot aggregate."
+		boot_hash=$(expr substr $boot_aggregate 16 40)
+		if [ "${ima_hash}" = "${boot_hash}" ]; then
+			tst_res TPASS "bios aggregate matches IMA boot aggregate"
 		else
-			tst_resm TFAIL "bios aggregate does not match IMA boot aggregate."
+			tst_res TFAIL "bios aggregate does not match IMA boot aggregate"
 		fi
 	fi
 }
@@ -74,64 +61,53 @@ test01()
 # the PCR values from /sys/devices.
 validate_pcr()
 {
-	ima_measurements=$SECURITYFS/ima/binary_runtime_measurements
-	aggregate_pcr=$(ima_measure $ima_measurements --validate)
-	dev_pcrs=$1
-	RC=0
+	tst_res TINFO "verify PCR (Process Control Register)"
+
+	local ima_measurements="$BINARY_MEASUREMENTS"
+	local aggregate_pcr="$(ima_measure $ima_measurements --validate)"
+	local dev_pcrs="$1"
+	local ret=0
 
-	while read line ; do
+	while read line; do
 		pcr=$(expr substr "${line}" 1 6)
 		if [ "${pcr}" = "PCR-10" ]; then
 			aggr=$(expr substr "${aggregate_pcr}" 26 59)
 			pcr=$(expr substr "${line}" 9 59)
-			[ "${pcr}" = "${aggr}" ] || RC=$?
+			[ "${pcr}" = "${aggr}" ] || ret=$?
 		fi
 	done < $dev_pcrs
-	return $RC
+	return $ret
 }
 
-# Function:     test02
-# Description	- Verify ima calculated aggregate PCR values matches
-#		  actual PCR value.
-test02()
+test2()
 {
+	tst_res TINFO "verify PCR values"
 
-	# Would be nice to know where the PCRs are located.  Is this safe?
-	PCRS_PATH=$(find /$SYSFS/devices/ | grep pcrs)
+	# Would be nice to know where the PCRs are located. Is this safe?
+	local pcrs_path="$(find $SYSFS/devices/ | grep pcrs)"
 	if [ $? -eq 0 ]; then
-		validate_pcr $PCRS_PATH
+		validate_pcr $pcrs_path
 		if [ $? -eq 0 ]; then
-			tst_resm TPASS "aggregate PCR value matches real PCR value."
+			tst_res TPASS "aggregate PCR value matches real PCR value"
 		else
-			tst_resm TFAIL "aggregate PCR value does not match real PCR value."
+			tst_res TFAIL "aggregate PCR value does not match real PCR value"
 		fi
 	else
-		tst_resm TFAIL "TPM not enabled, no PCR value to validate"
+		tst_res TCONF "TPM not enabled, no PCR value to validate"
 	fi
 }
 
-# Function:     test03
-# Description 	- Verify template hash value for IMA entry is correct.
-test03()
+test3()
 {
+	tst_res TINFO "verify template hash value"
 
-	ima_measurements=$SECURITYFS/ima/binary_runtime_measurements
-	aggregate_pcr=$(ima_measure $ima_measurements --verify --validate) > /dev/null
+	local ima_measurements="$BINARY_MEASUREMENTS"
+	ima_measure $ima_measurements --verify --validate
 	if [ $? -eq 0 ]; then
-		tst_resm TPASS "verified IMA template hash values."
+		tst_res TPASS "verified IMA template hash values"
 	else
-		tst_resm TFAIL "error verifing IMA template hash values."
+		tst_res TFAIL "error verifing IMA template hash values"
 	fi
 }
 
-. ima_setup.sh
-
-setup
-TST_CLEANUP=cleanup
-
-init
-test01
-test02
-test03
-
-tst_exit
+tst_run
diff --git a/testcases/kernel/security/integrity/ima/tests/ima_violations.sh b/testcases/kernel/security/integrity/ima/tests/ima_violations.sh
index 1b86b5f1a..0e9afa7ff 100755
--- a/testcases/kernel/security/integrity/ima/tests/ima_violations.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_violations.sh
@@ -1,44 +1,47 @@
 #!/bin/sh
-################################################################################
-##                                                                            ##
-## Copyright (C) 2009 IBM Corporation                                         ##
-##                                                                            ##
-## This program is free software;  you can redistribute it and#or modify      ##
-## it under the terms of the GNU General Public License as published by       ##
-## the Free Software Foundation; either version 2 of the License, or          ##
-## (at your option) any later version.                                        ##
-##                                                                            ##
-## This program is distributed in the hope that it will be useful, but        ##
-## WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY ##
-## or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License   ##
-## for more details.                                                          ##
-##                                                                            ##
-## You should have received a copy of the GNU General Public License          ##
-## along with this program;  if not, write to the Free Software               ##
-## Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA    ##
-##                                                                            ##
-################################################################################
+# Copyright (c) 2009 IBM Corporation
+# Copyright (c) 2018 Petr Vorel <pvorel@suse.cz>
 #
-# File :        ima_violations.sh
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation; either version 2 of
+# the License, or (at your option) any later version.
 #
-# Description:  This file tests ToMToU and open_writer violations invalidate
-#		the PCR and are logged.
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
 #
-# Author:       Mimi Zohar, zohar@ibm.vnet.ibm.com
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
 #
-# Return        - zero on success
-#               - non zero on failure. return value from commands ($RC)
-################################################################################
+# Author: Mimi Zohar, zohar@ibm.vnet.ibm.com
+#
+# Test whether ToMToU and open_writer violations invalidatethe PCR and are logged.
 
-export TST_TOTAL=3
-export TCID="ima_violations"
+TST_SETUP="setup"
+TST_CNT=3
 
-open_file_read()
+. ima_setup.sh
+. daemonlib.sh
+
+setup()
 {
-	exec 3< $1
-	if [ $? -ne 0 ]; then
-		exit 1
+	FILE="test.txt"
+	IMA_VIOLATIONS="$SECURITYFS/ima/violations"
+	LOG="/var/log/messages"
+
+	if status_daemon auditd; then
+		LOG="/var/log/audit/audit.log"
 	fi
+	[ -f "$LOG" ] || \
+		tst_brk TBROK "log $LOG does not exist (bug in detection?)"
+	tst_res TINFO "using log $LOG"
+}
+
+open_file_read()
+{
+	exec 3< $FILE || exit 1
 }
 
 close_file_read()
@@ -48,11 +51,8 @@ close_file_read()
 
 open_file_write()
 {
-	exec 4> $1
-	if [ $? -ne 0 ]; then
-		exit 1
-	echo 'testing, testing, ' >&4
-	fi
+	exec 4> $FILE || exit 1
+	echo 'test writing' >&4
 }
 
 close_file_write()
@@ -60,103 +60,95 @@ close_file_write()
 	exec 4>&-
 }
 
-init()
+get_count()
 {
-	service auditd status > /dev/null 2>&1
-	if [ $? -ne 0 ]; then
-		log=/var/log/messages
-	else
-		log=/var/log/audit/audit.log
-		tst_resm TINFO "requires integrity auditd patch"
-	fi
+	local search="$1"
+	echo $(grep -c "${search}.*${FILE}" $LOG)
+}
 
-	ima_violations=$SECURITYFS/ima/violations
+validate()
+{
+	local num_violations="$1"
+	local count="$2"
+	local search="$3"
+	local max_attempt=3
+	local count2 i num_violations_new
+
+	for i in $(seq 1 $max_attempt); do
+		read num_violations_new < $IMA_VIOLATIONS
+		count2="$(get_count $search)"
+		if [ $(($num_violations_new - $num_violations)) -gt 0 ]; then
+			if [ $count2 -gt $count ]; then
+				tst_res TPASS "$search violation added"
+				return
+			else
+				tst_res TINFO "$search not found in $LOG ($i/$max_attempt attempt)..."
+				tst_sleep 1s
+			fi
+		else
+			tst_res TFAIL "$search violation not added"
+			return
+		fi
+	done
+	tst_res TFAIL "$search not found in $LOG"
 }
 
-# Function:     test01
-# Description	- Verify open writers violation
-test01()
+test1()
 {
-	read num_violations < $ima_violations
+	tst_res TINFO "verify open writers violation"
 
-	TMPFN=test.txt
-	open_file_write $TMPFN
-	open_file_read $TMPFN
+	local search="open_writers"
+	local count num_violations
+
+	read num_violations < $IMA_VIOLATIONS
+	count="$(get_count $search)"
+
+	open_file_write
+	open_file_read
 	close_file_read
 	close_file_write
-	read num_violations_new < $ima_violations
-	num=$(($(expr $num_violations_new - $num_violations)))
-	if [ $num -gt 0 ]; then
-		tail $log | grep test.txt | grep -q 'open_writers'
-		if [ $? -eq 0 ]; then
-			tst_resm TPASS "open_writers violation added(test.txt)"
-		else
-			tst_resm TFAIL "(message ratelimiting?)"
-		fi
-	else
-		tst_resm TFAIL "open_writers violation not added(test.txt)"
-	fi
+
+	validate $num_violations $count $search
 }
 
-# Function:     test02
-# Description   - Verify ToMToU violation
-test02()
+test2()
 {
-	read num_violations < $ima_violations
+	tst_res TINFO "verify ToMToU violation"
+
+	local search="ToMToU"
+	local count num_violations
 
-	TMPFN=test.txt
-	open_file_read $TMPFN
-	open_file_write $TMPFN
+	read num_violations < $IMA_VIOLATIONS
+	count="$(get_count $search)"
+
+	open_file_read
+	open_file_write
 	close_file_write
 	close_file_read
-	read num_violations_new < $ima_violations
-	num=$(($(expr $num_violations_new - $num_violations)))
-	if [ $num -gt 0 ]; then
-		tail $log | grep test.txt | grep -q 'ToMToU'
-		if [ $? -eq 0 ]; then
-			tst_resm TPASS "ToMToU violation added(test.txt)"
-		else
-			tst_resm TFAIL "(message ratelimiting?)"
-		fi
-	else
-		tst_resm TFAIL "ToMToU violation not added(test.txt)"
-	fi
+
+	validate $num_violations $count $search
 }
 
-# Function:     test03
-# Description 	- verify open_writers using mmapped files
-test03()
+test3()
 {
-	read num_violations < $ima_violations
-
-	TMPFN=test.txtb
-	echo 'testing testing ' > $TMPFN
-	ima_mmap $TMPFN & p1=$!
-	sleep 1		# got to wait for ima_mmap to mmap the file
-	open_file_read $TMPFN
-	read num_violations_new < $ima_violations
-	num=$(($(expr $num_violations_new - $num_violations)))
-	if [ $num -gt 0 ]; then
-		tail $log | grep test.txtb | grep -q 'open_writers'
-		if [ $? -eq 0 ]; then
-			tst_resm TPASS "mmapped open_writers violation added(test.txtb)"
-		else
-			tst_resm TFAIL "(message ratelimiting?)"
-		fi
-	else
-		tst_resm TFAIL "mmapped open_writers violation not added(test.txtb)"
-	fi
-	close_file_read
-}
+	tst_res TINFO "verify open_writers using mmapped files"
 
-. ima_setup.sh
+	local search="open_writers"
+	local count num_violations
+
+	read num_violations < $IMA_VIOLATIONS
+	count="$(get_count $search)"
 
-setup
-TST_CLEANUP=cleanup
+	echo 'testing testing' > $FILE
 
-init
-test01
-test02
-test03
+	ima_mmap $FILE &
+	# wait for violations appear in logs
+	tst_sleep 1s
+
+	open_file_read
+	close_file_read
+
+	validate $num_violations $count $search
+}
 
-tst_exit
+tst_run
-- 
2.16.3

Re: [RFC PATCH v3 01/10] security/ima: Rewrite tests into new API + fixes

[Mimi Zohar]

On Thu, 2018-04-19 at 21:54 +0200, Petr Vorel wrote:

> -# Function:     test02
> -# Description	- Verify modifying, then reading, a file causes a new
> -# 		  measurement to be added to the IMA measurement list.
> -test02()
> +ima_check()
>  {
> -	# Modify test.txt
> -	echo $(date) - file modified >> test.txt
> +	local digest="$DEFAULT_DIGEST_OLD_FORMAT"
> +	local hash expected_hash line
> +
> +	# need to read file to get updated $ASCII_MEASUREMENTS
> +	cat $TEST_FILE > /dev/null
> +
> +	line="$(grep $TEST_FILE $ASCII_MEASUREMENTS | tail -1)"
> +	[ -n "$line" ] || tst_res TFAIL "cannot find measurement for '$TEST_FILE'"
> 
> -	# Calculating the sha1sum of test.txt should add
> -	# the new measurement to the measurement list
> -	hash=$(sha1sum test.txt | sed 's/  -//')
> +	[ "$DIGEST_INDEX" ] && digest="$(echo "$line" | awk '{print $(NF-'$DIGEST_INDEX')}' | cut -d ':' -f 1)"
> +	hash="$(echo "$line" | awk '{print $(NF-1)}' | cut -d ':' -f 2)"

With the "ima-sig" template, with a measurement that does not contain
the signature, this works fine.  There's a problem with lines
containing the signature.

Sample ima-sig template measurements with/without the signature:
line="10 ee788468d1b416a394feb9f4e5650302d9cd5574 ima-sig sha256:866c2542efd5c7528591eb3bb2861a1994a655da47732ccf28f7f4b1ce42d564 /usr/lib64/libpam.so.0.84.1"

line="10 d3afb4df5fe42485b99677f4b68a04692977b4bc ima-sig sha256:7b85508c9181670fe169935310b8c95d7c2573f0318a70cecd12868569aab891 /etc/profile.d/less.sh 0302046e6c104601008bd533707b34a9e896d3d530a88e9af517fb7e8cf79e9e55064a577fcbcdb81236ede6fec0638d357e4c2ed9b261320f8789378d1e58af8e1c6f40ebdf080759be2c633b27bc8aed85af0620fa27700c68fdf31d33b2f9e36432a1e7d7eb8dbf20b9474d332deb9697767ee13e13c116544a843b54fce842d24ea485bb41f6f7b1e9fa3faed0c591f5243cee008b9499e48064141662d3c4d002b07448ae54dc8d8674437143d73c4e34f5b416300ba890dc391eae9e5b1e89190790d0ea77d1dc57e07dae9ca003294a36fda09c31f8afa347701bfcf5aed0fda9cf7a37f734ba80fc10f2d60409f0beba532f3e5cc15ae995128e466b20fdadef789e285519"

> 
> -	# Check if the new measurement exists
> -	cat /sys/kernel/security/ima/ascii_runtime_measurements > measurements
> -	$(grep $hash measurements > /dev/null)
> +	tst_res TINFO "computing hash for $digest digest"
> +	expected_hash="$(compute_hash $digest $TEST_FILE)" || \
> +		{ tst_res TCONF "cannot compute hash for '$digest' digest"; return; }
> 
> -	if [ $? -ne 0 ]; then
> -		tst_resm TFAIL "Modified file not measured"
> -		tst_resm TINFO "iversion not supported; or not mounted with iversion"
> +	if [ "$hash" = "$expected_hash" ]; then
> +		tst_res TPASS "correct hash found"
>  	else
> -		tst_resm TPASS "Modified file measured"
> +		tst_res TFAIL "hash not found"
>  	fi
>  }
> 

Re: [RFC PATCH v3 01/10] security/ima: Rewrite tests into new API + fixes

[Petr Vorel]

Hi Mimi,

> > +ima_check()
...
> > +	[ "$DIGEST_INDEX" ] && digest="$(echo "$line" | awk '{print $(NF-'$DIGEST_INDEX')}' | cut -d ':' -f 1)"
> > +	hash="$(echo "$line" | awk '{print $(NF-1)}' | cut -d ':' -f 2)"

> With the "ima-sig" template, with a measurement that does not contain
> the signature, this works fine.  There's a problem with lines
> containing the signature.

> Sample ima-sig template measurements with/without the signature:
> line="10 ee788468d1b416a394feb9f4e5650302d9cd5574 ima-sig sha256:866c2542efd5c7528591eb3bb2861a1994a655da47732ccf28f7f4b1ce42d564 /usr/lib64/libpam.so.0.84.1"

> line="10 d3afb4df5fe42485b99677f4b68a04692977b4bc ima-sig sha256:7b85508c9181670fe169935310b8c95d7c2573f0318a70cecd12868569aab891 /etc/profile.d/less.sh 0302046e6c104601008bd533707b34a9e896d3d530a88e9af517fb7e8cf79e9e55064a577fcbcdb81236ede6fec0638d357e4c2ed9b261320f8789378d1e58af8e1c6f40ebdf080759be2c633b27bc8aed85af0620fa27700c68fdf31d33b2f9e36432a1e7d7eb8dbf20b9474d332deb9697767ee13e13c116544a843b54fce842d24ea485bb41f6f7b1e9fa3faed0c591f5243cee008b9499e48064141662d3c4d002b07448ae54dc8d8674437143d73c4e34f5b416300ba890dc391eae9e5b1e89190790d0ea77d1dc57e07dae9ca003294a36fda09c31f8afa347701bfcf5aed0fda9cf7a37f734ba80fc10f2d60409f0beba532f3e5cc15ae995128e466b20fdadef789e285519"

Sorry, I haven't setup machine with IMA signature support yet. So booting with
ima_template_fmt=d-ng|n-ng|sig (or kernel with CONFIG_IMA_DEFAULT_TEMPLATE="ima-sig")
without any keys generated with evmctl obviously doesn't bring any signatures.

It could be a solution to detect presence of signature for 'ima-sig' with simple counting
parameters (5: no signature, 6: signature when ima_template_fmt is not used).  And good
thing is that line without signature is different: signature part isn't left, but there is
and space (' ') for it.

The detection of both indexes (the hash itself and the digest) needs to be bit smarter
anyway as imagine someone crazy using ima_template_fmt=d-ng|n-ng|sig|d-ng|n-ng|sig
parameter.

Kind regards,
Petr

[RFC PATCH v3 02/10] security/ima: Change order of tests

[Petr Vorel]

Unfortunately in some circumstances there are interdependencies between
tests.
measurements test require loaded IMA policy. If it's not loaded, policy
test do it for us => run measurements test after policy test.

Policy test somehow breaks violations test => run it before policy test.
TODO: this does not help if CONFIG_IMA_WRITE_POLICY=y and without auditd
daemon. Maybe we should require auditd for violation tests.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
Is it caused by using unsigned policy?
This problem haven't been solved by avoiding tmpfs.
---
 runtest/ima | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/runtest/ima b/runtest/ima
index bcae16bb7..e7824a62a 100644
--- a/runtest/ima
+++ b/runtest/ima
@@ -1,5 +1,5 @@
 #DESCRIPTION:Integrity Measurement Architecture (IMA)
-ima_measurements ima_measurements.sh
+ima_violations ima_violations.sh
 ima_policy ima_policy.sh
+ima_measurements ima_measurements.sh
 ima_tpm ima_tpm.sh
-ima_violations ima_violations.sh
-- 
2.16.3

[RFC PATCH v3 03/10] ima/ima_policy.sh: Improve check of policy writability

[Petr Vorel]

+ merge test3 into test2 as we test multiple writes already in test2.

Kernel without CONFIG_IMA_WRITE_POLICY is not possible to repeat writing
into policy. Add check to TCONF in this case.

It's not easy to detect disabled CONFIG_IMA_WRITE_POLICY for different
behavior across kernel versions.
On older kernels (before CONFIG_IMA_WRITE_POLICY enabled) or on new ones
with enabled both CONFIG_IMA_READ_POLICY and CONFIG_IMA_WRITE_POLICY
policy file after writing disappears.

Kernels with enabled CONFIG_IMA_READ_POLICY and (regardless of
CONFIG_IMA_WRITE_POLICY) keeps policy file with the same permissions
600. The only way to detect is is to echo empty string into policy and
detect errno:

       | OLD    | WRITE       | READ && !WRITE | !READ && !WRITE
------------------------------------------------------------------
before | ENOENT | exit code 0 | exit code 0    | exit code 0
after  | EACCES | exit code 0 | EBUSY	       | EACCES

OLD: kernels before CONFIG_IMA_WRITE_POLICY introduced (kernel < 4.5)
READ: CONFIG_IMA_READ_POLICY
WRITE: CONFIG_IMA_WRITE_POLICY

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

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 .../security/integrity/ima/tests/ima_policy.sh     | 38 ++++++++++------------
 1 file changed, 17 insertions(+), 21 deletions(-)

diff --git a/testcases/kernel/security/integrity/ima/tests/ima_policy.sh b/testcases/kernel/security/integrity/ima/tests/ima_policy.sh
index 2efa90038..35eb4055b 100755
--- a/testcases/kernel/security/integrity/ima/tests/ima_policy.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_policy.sh
@@ -20,15 +20,24 @@
 # Test replacing the default integrity measurement policy.
 
 TST_SETUP="setup"
-TST_CNT=3
+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 /sys/kernel/security/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()
 {
 	IMA_POLICY="$IMA_DIR/policy"
-	[ -f $IMA_POLICY ] || \
-		tst_brk TCONF "IMA policy already loaded and kernel not configured to enable multiple writes it"
+	check_policy_writable
 
 	VALID_POLICY="$TST_DATAROOT/measure.policy"
 	[ -f $VALID_POLICY ] || tst_brk TCONF "missing $VALID_POLICY"
@@ -68,6 +77,7 @@ test1()
 
 	local p1
 
+	check_policy_writable
 	load_policy $INVALID_POLICY & p1=$!
 	wait "$p1"
 	if [ $? -ne 0 ]; then
@@ -79,10 +89,11 @@ test1()
 
 test2()
 {
-	tst_res TINFO "verify that policy file is not opened concurrently"
+	tst_res TINFO "verify that policy file is not opened concurrently and able to loaded multiple times"
 
 	local p1 p2 rc1 rc2
 
+	check_policy_writable
 	load_policy $VALID_POLICY & p1=$!
 	load_policy $VALID_POLICY & p2=$!
 	wait "$p1"; rc1=$?
@@ -90,24 +101,9 @@ test2()
 	if [ $rc1 -eq 0 ] && [ $rc2 -eq 0 ]; then
 		tst_res TFAIL "policy opened concurrently"
 	elif [ $rc1 -eq 0 ] || [ $rc2 -eq 0 ]; then
-		tst_res TPASS "policy was loaded just by one process"
-	else
-		tst_res TFAIL "problem loading policy"
-	fi
-}
-
-test3()
-{
-	tst_res TINFO "verify that invalid policy isn't loaded"
-
-	local p1
-
-	load_policy $INVALID_POLICY & p1=$!
-	wait "$p1"
-	if [ $? -ne 0 ]; then
-		tst_res TPASS "didn't replace valid policy"
+		tst_res TPASS "policy was loaded just by one process and able to loaded multiple times"
 	else
-		tst_res TFAIL "replaced valid policy"
+		tst_res TFAIL "problem with loading policy (policy should be able to load multiple times)"
 	fi
 }
 
-- 
2.16.3

[RFC PATCH v3 04/10] ima/ima_policy.sh: Load whole policy with cat

[Petr Vorel]

Originally writing the policy was done one rule at a time, but that's
not required since kernel 2.6.35 (6ccd04563005 "ima: handle multiple rules per write")

Signed-off-by: Petr Vorel <pvorel@suse.cz>
Suggested-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
 testcases/kernel/security/integrity/ima/tests/ima_policy.sh | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/testcases/kernel/security/integrity/ima/tests/ima_policy.sh b/testcases/kernel/security/integrity/ima/tests/ima_policy.sh
index 35eb4055b..1c4a0b922 100755
--- a/testcases/kernel/security/integrity/ima/tests/ima_policy.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_policy.sh
@@ -53,17 +53,9 @@ load_policy()
 	exec 2>/dev/null 4>$IMA_POLICY
 	[ $? -eq 0 ] || exit 1
 
-	cat $1 |
-	while read line; do
-		if [ "${line#\#}" = "${line}" ]; then
-			echo "$line" >&4 2> /dev/null
-			if [ $? -ne 0 ]; then
-				exec 4>&-
-				return 1
-			fi
-		fi
-	done
+	cat $1 >&4 2> /dev/null
 	ret=$?
+	exec 4>&-
 
 	[ $ret -eq 0 ] && \
 		tst_res TINFO "IMA policy updated, please reboot after testing to restore settings"
-- 
2.16.3

[RFC PATCH v3 05/10] ima/ima_boot_aggregate: Increase MAX_EVENT_SIZE to 1MB

[Petr Vorel]

This is needed as according IMA developers there are BIOS events larger
than 4k [1]. Actual size for TPM 1.2 is undefined, TPM 2.0 specifies:
"For software parsing the event log, the parser can choose an arbitrary
maximum size, but this specification recommends a maximum value for the
TCG_PCR_EVENT2.eventSize field of 1MB." [2].

So lets follow the specification and allocate 1MB.

[1] http://lists.linux.it/pipermail/ltp/2018-January/006970.html
[2] http://lists.linux.it/pipermail/ltp/2018-January/007002.html

Suggested-by: George Wilson <gcwilson@us.ibm.com>
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 .../security/integrity/ima/src/ima_boot_aggregate.c      | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c b/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
index f7ae77cb1..862cc07ba 100644
--- a/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
+++ b/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
@@ -21,6 +21,7 @@
 #include <fcntl.h>
 #include <string.h>
 #include <unistd.h>
+#include <limits.h>
 
 #include "config.h"
 #include "test.h"
@@ -30,7 +31,7 @@ char *TCID = "ima_boot_aggregate";
 #if HAVE_LIBCRYPTO
 #include <openssl/sha.h>
 
-#define MAX_EVENT_SIZE 500
+#define MAX_EVENT_SIZE (1024*1024)
 #define EVENT_HEADER_SIZE 32
 #define MAX_EVENT_DATA_SIZE (MAX_EVENT_SIZE - EVENT_HEADER_SIZE)
 #define NUM_PCRS 8		/*  PCR registers 0-7 in boot aggregate */
@@ -56,7 +57,7 @@ int main(int argc, char *argv[])
 			unsigned char digest[SHA_DIGEST_LENGTH];
 			u_int16_t len;
 		} header;
-		unsigned char data[MAX_EVENT_DATA_SIZE];
+		char *data;
 	} event;
 	struct {
 		unsigned char digest[SHA_DIGEST_LENGTH];
@@ -80,6 +81,12 @@ int main(int argc, char *argv[])
 	for (i = 0; i < NUM_PCRS; i++)
 		memset(&pcr[i].digest, 0, SHA_DIGEST_LENGTH);
 
+	event.data = (char *) malloc(MAX_EVENT_DATA_SIZE);
+	if (!event.data) {
+		printf("Cannot allocate memory\n");
+		return 1;
+	}
+
 	/* Extend the pseudo PCRs with the event digest */
 	while (fread(&event, sizeof(event.header), 1, fp)) {
 		if (debug) {
@@ -90,13 +97,16 @@ int main(int argc, char *argv[])
 		SHA1_Update(&c, pcr[event.header.pcr].digest, 20);
 		SHA1_Update(&c, event.header.digest, 20);
 		SHA1_Final(pcr[event.header.pcr].digest, &c);
+#if MAX_EVENT_DATA_SIZE < USHRT_MAX
 		if (event.header.len > MAX_EVENT_DATA_SIZE) {
-			printf("Error event too long");
+			printf("Error event too long\n");
 			break;
 		}
+#endif
 		fread(event.data, event.header.len, 1, fp);
 	}
 	fclose(fp);
+	free(event.data);
 
 	/* Extend the boot aggregate with the pseudo PCR digest values */
 	memset(&boot_aggregate, 0, SHA_DIGEST_LENGTH);
-- 
2.16.3

Re: [LTP] [RFC PATCH v3 05/10] ima/ima_boot_aggregate: Increase MAX_EVENT_SIZE to 1MB

[Cyril Hrubis]

Hi!
> +	event.data = (char *) malloc(MAX_EVENT_DATA_SIZE);
                        ^
		Please never cast return value from malloc() in	C.

The malloc returns void* which is compatible with any other pointer type
for assigments. This is only needed when you attempt to use malloc()
in C++, but that is not the case here.

-- 
Cyril Hrubis
chrubis@suse.cz

[RFC PATCH v3 06/10] ima/tpm.sh: Use evmctl + other fixes

[Petr Vorel]

* Improve TCONF "no TMP support" messages

test1
* Fix reading boot_aggregate for ima-ng

test2
* Fix pcrs paths
* Drop ima_measure binary, use upstream tool evmctl from ima-evm-utils instead
https://git.code.sf.net/p/linux-ima/ima-evm-utils
* Check evmctl in test2 (if it's missing test1 is still being run)

test3
* Dropped, as evmctl has no 'ima_measure --validate` equivalent

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 testcases/kernel/security/integrity/.gitignore     |   1 -
 .../security/integrity/ima/src/ima_measure.c       | 219 ---------------------
 .../kernel/security/integrity/ima/tests/ima_tpm.sh |  67 ++++---
 3 files changed, 33 insertions(+), 254 deletions(-)
 delete mode 100644 testcases/kernel/security/integrity/ima/src/ima_measure.c

diff --git a/testcases/kernel/security/integrity/.gitignore b/testcases/kernel/security/integrity/.gitignore
index 1759bc98b..184aa78ce 100644
--- a/testcases/kernel/security/integrity/.gitignore
+++ b/testcases/kernel/security/integrity/.gitignore
@@ -1,3 +1,2 @@
 /ima/src/ima_boot_aggregate
-/ima/src/ima_measure
 /ima/src/ima_mmap
diff --git a/testcases/kernel/security/integrity/ima/src/ima_measure.c b/testcases/kernel/security/integrity/ima/src/ima_measure.c
deleted file mode 100644
index 3aa56490f..000000000
--- a/testcases/kernel/security/integrity/ima/src/ima_measure.c
+++ /dev/null
@@ -1,219 +0,0 @@
-/*
- * Copyright (c) International Business Machines  Corp., 2008
- *
- * Authors:
- * Reiner Sailer <sailer@watson.ibm.com>
- * Mimi Zohar <zohar@us.ibm.com>
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License as
- * published by the Free Software Foundation, version 2 of the
- * License.
- *
- * File: ima_measure.c
- *
- * Calculate the SHA1 aggregate-pcr value based on the IMA runtime
- * binary measurements.
- */
-#include <stdio.h>
-#include <stdlib.h>
-#include <sys/types.h>
-#include <sys/stat.h>
-#include <fcntl.h>
-#include <string.h>
-#include <unistd.h>
-
-#include "config.h"
-#include "test.h"
-
-char *TCID = "ima_measure";
-
-#if HAVE_LIBCRYPTO
-#include <openssl/sha.h>
-
-#define TCG_EVENT_NAME_LEN_MAX	255
-
-int TST_TOTAL = 1;
-
-static int verbose;
-
-#define print_info(format, arg...) \
-	if (verbose) \
-		printf(format, ##arg)
-
-static u_int8_t zero[SHA_DIGEST_LENGTH];
-static u_int8_t fox[SHA_DIGEST_LENGTH];
-
-struct event {
-	struct {
-		u_int32_t pcr;
-		u_int8_t digest[SHA_DIGEST_LENGTH];
-		u_int32_t name_len;
-	} header;
-	char name[TCG_EVENT_NAME_LEN_MAX + 1];
-	struct {
-		u_int8_t digest[SHA_DIGEST_LENGTH];
-		char filename[TCG_EVENT_NAME_LEN_MAX + 1];
-	} ima_data;
-	int filename_len;
-};
-
-static void display_sha1_digest(u_int8_t * digest)
-{
-	int i;
-
-	for (i = 0; i < 20; i++)
-		print_info(" %02X", (*(digest + i) & 0xff));
-}
-
-/*
- * Calculate the sha1 hash of data
- */
-static void calc_digest(u_int8_t * digest, int len, void *data)
-{
-	SHA_CTX c;
-
-	/* Calc template hash for an ima entry */
-	memset(digest, 0, sizeof *digest);
-	SHA1_Init(&c);
-	SHA1_Update(&c, data, len);
-	SHA1_Final(digest, &c);
-}
-
-static int verify_template_hash(struct event *template)
-{
-	int rc;
-
-	rc = memcmp(fox, template->header.digest, sizeof fox);
-	if (rc != 0) {
-		u_int8_t digest[SHA_DIGEST_LENGTH];
-
-		memset(digest, 0, sizeof digest);
-		calc_digest(digest, sizeof template->ima_data,
-			    &template->ima_data);
-		rc = memcmp(digest, template->header.digest, sizeof digest);
-		return rc != 0 ? 1 : 0;
-	}
-	return 0;
-}
-
-/*
- * ima_measurements.c - calculate the SHA1 aggregate-pcr value based
- * on the IMA runtime binary measurements.
- *
- * format: ima_measurement [--validate] [--verify] [--verbose]
- *
- * --validate: forces validation of the aggregrate pcr value
- * 	     for an invalidated PCR. Replace all entries in the
- * 	     runtime binary measurement list with 0x00 hash values,
- * 	     which indicate the PCR was invalidated, either for
- * 	     "a time of measure, time of use"(ToMToU) error, or a
- *	     file open for read was already open for write, with
- * 	     0xFF's hash value, when calculating the aggregate
- *	     pcr value.
- *
- * --verify: for all IMA template entries in the runtime binary
- * 	     measurement list, calculate the template hash value
- * 	     and compare it with the actual template hash value.
- *	     Return the number of incorrect hash measurements.
- *
- * --verbose: For all entries in the runtime binary measurement
- *	     list, display the template information.
- *
- * template info:  list #, PCR-register #, template hash, template name
- *	IMA info:  IMA hash, filename hint
- *
- * Ouput: displays the aggregate-pcr value
- * Return code: if verification enabled, returns number of verification
- * 		errors.
- */
-int main(int argc, char *argv[])
-{
-	FILE *fp;
-	struct event template;
-	u_int8_t pcr[SHA_DIGEST_LENGTH];
-	int i, count = 0;
-
-	int validate = 0;
-	int verify = 0;
-
-	if (argc < 2) {
-		printf("format: %s binary_runtime_measurements"
-		       " [--validate] [--verbose] [--verify]\n", argv[0]);
-		return 1;
-	}
-
-	for (i = 2; i < argc; i++) {
-		if (strncmp(argv[i], "--validate", 8) == 0)
-			validate = 1;
-		if (strncmp(argv[i], "--verbose", 7) == 0)
-			verbose = 1;
-		if (strncmp(argv[i], "--verify", 6) == 0)
-			verify = 1;
-	}
-
-	fp = fopen(argv[1], "r");
-	if (!fp) {
-		printf("fn: %s\n", argv[1]);
-		perror("Unable to open file\n");
-		return 1;
-	}
-	memset(pcr, 0, SHA_DIGEST_LENGTH);	/* initial PCR content 0..0 */
-	memset(zero, 0, SHA_DIGEST_LENGTH);
-	memset(fox, 0xff, SHA_DIGEST_LENGTH);
-
-	print_info("### PCR HASH                                  "
-		   "TEMPLATE-NAME\n");
-	while (fread(&template.header, sizeof template.header, 1, fp)) {
-		SHA_CTX c;
-
-		/* Extend simulated PCR with new template digest */
-		SHA1_Init(&c);
-		SHA1_Update(&c, pcr, SHA_DIGEST_LENGTH);
-		if (validate) {
-			if (memcmp(template.header.digest, zero, 20) == 0)
-				memset(template.header.digest, 0xFF, 20);
-		}
-		SHA1_Update(&c, template.header.digest, 20);
-		SHA1_Final(pcr, &c);
-
-		print_info("%3d %03u ", count++, template.header.pcr);
-		display_sha1_digest(template.header.digest);
-		if (template.header.name_len > TCG_EVENT_NAME_LEN_MAX) {
-			printf("%d ERROR: event name too long!\n",
-			       template.header.name_len);
-			exit(1);
-		}
-		memset(template.name, 0, sizeof template.name);
-		fread(template.name, template.header.name_len, 1, fp);
-		print_info(" %s ", template.name);
-
-		memset(&template.ima_data, 0, sizeof template.ima_data);
-		fread(&template.ima_data.digest,
-		      sizeof template.ima_data.digest, 1, fp);
-		display_sha1_digest(template.ima_data.digest);
-
-		fread(&template.filename_len,
-		      sizeof template.filename_len, 1, fp);
-		fread(template.ima_data.filename, template.filename_len, 1, fp);
-		print_info(" %s\n", template.ima_data.filename);
-
-		if (verify)
-			if (verify_template_hash(&template) != 0) {
-				tst_resm(TFAIL, "Hash failed");
-			}
-	}
-	fclose(fp);
-
-	verbose = 1;
-	print_info("PCRAggr (re-calculated):");
-	display_sha1_digest(pcr);
-	tst_exit();
-}
-
-#else
-int main(void)
-{
-	tst_brkm(TCONF, NULL, "test requires libcrypto and openssl development packages");
-}
-#endif
diff --git a/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh b/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
index ed45ab8d2..0124c338f 100755
--- a/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
@@ -19,8 +19,8 @@
 #
 # Verify the boot and PCR aggregates.
 
-TST_NEEDS_CMDS="ima_boot_aggregate ima_measure"
-TST_CNT=3
+TST_CNT=2
+TST_NEEDS_CMDS="awk cut ima_boot_aggregate"
 
 . ima_setup.sh
 
@@ -31,24 +31,23 @@ test1()
 	local zero="0000000000000000000000000000000000000000"
 	local tpm_bios="$SECURITYFS/tpm0/binary_bios_measurements"
 	local ima_measurements="$ASCII_MEASUREMENTS"
-	local boot_aggregate boot_hash ima_hash line
+	local boot_aggregate boot_hash line
 
 	# IMA boot aggregate
 	read line < $ima_measurements
-	ima_hash=$(expr substr "${line}" 49 40)
+	boot_hash=$(echo $line | awk '{print $(NF-1)}' | cut -d':' -f2)
 
 	if [ ! -f "$tpm_bios" ]; then
-		tst_res TINFO "TPM not builtin kernel, or TPM not enabled"
+		tst_res TINFO "TPM Hardware Support not enabled in kernel or no TPM chip found"
 
-		if [ "${ima_hash}" = "${zero}" ]; then
+		if [ "${boot_hash}" = "${zero}" ]; then
 			tst_res TPASS "bios boot aggregate is 0"
 		else
 			tst_res TFAIL "bios boot aggregate is not 0"
 		fi
 	else
-		boot_aggregate=$(ima_boot_aggregate $tpm_bios)
-		boot_hash=$(expr substr $boot_aggregate 16 40)
-		if [ "${ima_hash}" = "${boot_hash}" ]; then
+		boot_aggregate=$(ima_boot_aggregate $tpm_bios | grep "boot_aggregate:" | cut -d':' -f2)
+		if [ "${boot_hash}" = "${boot_aggregate}" ]; then
 			tst_res TPASS "bios aggregate matches IMA boot aggregate"
 		else
 			tst_res TFAIL "bios aggregate does not match IMA boot aggregate"
@@ -63,29 +62,42 @@ validate_pcr()
 {
 	tst_res TINFO "verify PCR (Process Control Register)"
 
-	local ima_measurements="$BINARY_MEASUREMENTS"
-	local aggregate_pcr="$(ima_measure $ima_measurements --validate)"
 	local dev_pcrs="$1"
-	local ret=0
+	local pcr hash aggregate_pcr
+
+	aggregate_pcr="$(evmctl -v ima_measurement $BINARY_MEASUREMENTS 2>&1 | \
+		grep 'HW PCR-10:' | awk '{print $3}')"
+	if [ -z "$aggregate_pcr" ]; then
+		tst_res TFAIL "failed to get PCR-10"
+		return
+	fi
 
 	while read line; do
-		pcr=$(expr substr "${line}" 1 6)
+		pcr="$(echo $line | cut -d':' -f1)"
 		if [ "${pcr}" = "PCR-10" ]; then
-			aggr=$(expr substr "${aggregate_pcr}" 26 59)
-			pcr=$(expr substr "${line}" 9 59)
-			[ "${pcr}" = "${aggr}" ] || ret=$?
+			hash="$(echo $line | cut -d':' -f2 | awk '{ gsub (" ", "", $0); print tolower($0) }')"
+			[ "${hash}" = "${aggregate_pcr}" ]
+			return $?
 		fi
 	done < $dev_pcrs
-	return $ret
+	return 1
 }
 
 test2()
 {
 	tst_res TINFO "verify PCR values"
+	tst_check_cmds evmctl
+
+	tst_res TINFO "evmctl version: $(evmctl --version)"
+
+	local pcrs_path="/sys/class/tpm/tpm0/device/pcrs"
+	if [ -f "$pcrs_path" ]; then
+		tst_res TINFO "new PCRS path, evmctl >= 1.1 required"
+	else
+		pcrs_path="/sys/class/misc/tpm0/device/pcrs"
+	fi
 
-	# Would be nice to know where the PCRs are located. Is this safe?
-	local pcrs_path="$(find $SYSFS/devices/ | grep pcrs)"
-	if [ $? -eq 0 ]; then
+	if [ -f "$pcrs_path" ]; then
 		validate_pcr $pcrs_path
 		if [ $? -eq 0 ]; then
 			tst_res TPASS "aggregate PCR value matches real PCR value"
@@ -93,20 +105,7 @@ test2()
 			tst_res TFAIL "aggregate PCR value does not match real PCR value"
 		fi
 	else
-		tst_res TCONF "TPM not enabled, no PCR value to validate"
-	fi
-}
-
-test3()
-{
-	tst_res TINFO "verify template hash value"
-
-	local ima_measurements="$BINARY_MEASUREMENTS"
-	ima_measure $ima_measurements --verify --validate
-	if [ $? -eq 0 ]; then
-		tst_res TPASS "verified IMA template hash values"
-	else
-		tst_res TFAIL "error verifing IMA template hash values"
+		tst_res TCONF "TPM Hardware Support not enabled in kernel or no TPM chip found"
 	fi
 }
 
-- 
2.16.3

[RFC PATCH v3 07/10] ima/ima_mmap: Reduce sleep + log it

[Petr Vorel]

Sleep reduced to 3s (30s is way too much).

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 testcases/kernel/security/integrity/ima/src/ima_mmap.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/testcases/kernel/security/integrity/ima/src/ima_mmap.c b/testcases/kernel/security/integrity/ima/src/ima_mmap.c
index 335f8525c..9045e79a0 100644
--- a/testcases/kernel/security/integrity/ima/src/ima_mmap.c
+++ b/testcases/kernel/security/integrity/ima/src/ima_mmap.c
@@ -25,6 +25,8 @@
 char *TCID = "ima_mmap";
 int TST_TOTAL = 1;
 
+#define SLEEP_AFTER_CLOSE 3
+
 int main(int argc, char *argv[])
 {
 	int fd;
@@ -47,7 +49,10 @@ int main(int argc, char *argv[])
 		return (-1);
 	}
 	close(fd);
-	sleep(30);
+
+	tst_resm(TINFO, "sleep %ds", SLEEP_AFTER_CLOSE);
+	sleep(SLEEP_AFTER_CLOSE);
+
 	if (munmap(file, 1024) < 0) {
 		perror("unmap");
 		return (-1);
-- 
2.16.3

Re: [LTP] [RFC PATCH v3 07/10] ima/ima_mmap: Reduce sleep + log it

[Cyril Hrubis]

Hi!
Proper synchronization between the processes wouldn't harm here.

FIY: we do have checkpoints for the shell library that can be used both
     from C sources as well as shell.

But given how big this patchset is at this point, it can wait for v2 I
guess.

-- 
Cyril Hrubis
chrubis@suse.cz

[RFC PATCH v3 08/10] ima/{ima_measurements,ima_violations}.sh: Avoid running on tmpfs

[Petr Vorel]

If $TMPDIR is on tmpfs, create loop device, format it to ext3 and run
tests in it.

The reason is that measure.policy excludes tmpfs (TMPFS_MAGIC,
"dont_measure fsmagic=0x01021994"), but TST_TMPDIR is often on tmpfs
filesystem. Lets test on ext3 created on loop device.

http://lists.linux.it/pipermail/ltp/2018-January/006970.html
http://lists.linux.it/pipermail/ltp/2018-March/007488.html

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 .../integrity/ima/tests/ima_measurements.sh        |  1 +
 .../security/integrity/ima/tests/ima_setup.sh      | 40 ++++++++++++++++++++--
 .../security/integrity/ima/tests/ima_violations.sh |  4 +++
 3 files changed, 42 insertions(+), 3 deletions(-)

diff --git a/testcases/kernel/security/integrity/ima/tests/ima_measurements.sh b/testcases/kernel/security/integrity/ima/tests/ima_measurements.sh
index 0bceeb71f..294e29d30 100755
--- a/testcases/kernel/security/integrity/ima/tests/ima_measurements.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_measurements.sh
@@ -22,6 +22,7 @@
 TST_NEEDS_CMDS="awk"
 TST_SETUP="setup"
 TST_CNT=3
+TST_NEEDS_DEVICE=1
 
 . ima_setup.sh
 
diff --git a/testcases/kernel/security/integrity/ima/tests/ima_setup.sh b/testcases/kernel/security/integrity/ima/tests/ima_setup.sh
index c08e2579e..03851167f 100644
--- a/testcases/kernel/security/integrity/ima/tests/ima_setup.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_setup.sh
@@ -28,6 +28,7 @@ TST_NEEDS_ROOT=1
 
 SYSFS="/sys"
 UMOUNT=
+FS_TYPE="ext3"
 
 mount_helper()
 {
@@ -39,15 +40,30 @@ mount_helper()
 	[ -n "$dir" ] && { echo "$dir"; return; }
 
 	if ! mkdir -p $default_dir; then
-		tst_brk TBROK "Failed to create $default_dir"
+		tst_brk TBROK "failed to create $default_dir"
 	fi
 	if ! mount -t $type $type $default_dir; then
-		tst_brk TBROK "Failed to mount $type"
+		tst_brk TBROK "failed to mount $type"
 	fi
 	UMOUNT="$default_dir $UMOUNT"
 	echo $default_dir
 }
 
+mount_loop_device()
+{
+	local ret
+
+	tst_check_cmds mkfs.$FS_TYPE
+	tst_mkfs $FS_TYPE $TST_DEVICE
+	ROD_SILENT mkdir -p mntpoint
+	mount ${TST_DEVICE} mntpoint
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		tst_brk TBROK "failed to mount device (mount exit = $ret)"
+	fi
+	cd mntpoint
+}
+
 ima_setup()
 {
 	SECURITYFS="$(mount_helper securityfs $SYSFS/kernel/security)"
@@ -57,7 +73,14 @@ ima_setup()
 	ASCII_MEASUREMENTS="$IMA_DIR/ascii_runtime_measurements"
 	BINARY_MEASUREMENTS="$IMA_DIR/binary_runtime_measurements"
 
-	[ -n "$TST_SETUP_CALLER" ] && $TST_SETUP_CALLER
+	if [ "$TST_NEEDS_DEVICE" = 1 ]; then
+		tst_res TINFO "\$TMPDIR is on tmpfs => run on loop device"
+		mount_loop_device
+	fi
+
+	if [ -n "$TST_SETUP_CALLER" ]; then
+		$TST_SETUP_CALLER
+	fi
 }
 
 ima_cleanup()
@@ -66,4 +89,15 @@ ima_cleanup()
 	for dir in $UMOUNT; do
 		umount $dir
 	done
+
+	if [ "$TST_NEEDS_DEVICE" = 1 ]; then
+		cd $TST_TMPDIR
+		tst_umount $TST_DEVICE
+	fi
 }
+
+# loop device is needed to use only for tmpfs
+TMPDIR="${TMPDIR:-/tmp}"
+if [ "$(df -T $TMPDIR | tail -1 | awk '{print $2}')" != "tmpfs" -a -n "$TST_NEEDS_DEVICE" ]; then
+	unset TST_NEEDS_DEVICE
+fi
diff --git a/testcases/kernel/security/integrity/ima/tests/ima_violations.sh b/testcases/kernel/security/integrity/ima/tests/ima_violations.sh
index 0e9afa7ff..8742f4593 100755
--- a/testcases/kernel/security/integrity/ima/tests/ima_violations.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_violations.sh
@@ -21,6 +21,7 @@
 
 TST_SETUP="setup"
 TST_CNT=3
+TST_NEEDS_DEVICE=1
 
 . ima_setup.sh
 . daemonlib.sh
@@ -149,6 +150,9 @@ test3()
 	close_file_read
 
 	validate $num_violations $count $search
+
+	# wait for ima_mmap to exit, so we can umount
+	tst_sleep 2s
 }
 
 tst_run
-- 
2.16.3

[RFC PATCH v3 09/10] ima: CRYPTO_LIBS are needed only for ima_boot_aggregate

[Petr Vorel]

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 testcases/kernel/security/integrity/ima/src/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/testcases/kernel/security/integrity/ima/src/Makefile b/testcases/kernel/security/integrity/ima/src/Makefile
index 0f4cf8c12..f7a818578 100644
--- a/testcases/kernel/security/integrity/ima/src/Makefile
+++ b/testcases/kernel/security/integrity/ima/src/Makefile
@@ -24,6 +24,6 @@ top_srcdir		?= ../../../../../..
 
 include $(top_srcdir)/include/mk/testcases.mk
 
-LDLIBS			+= $(CRYPTO_LIBS) -ldl
+ima_boot_aggregate: LDLIBS += $(CRYPTO_LIBS) -ldl
 
 include $(top_srcdir)/include/mk/generic_leaf_target.mk
-- 
2.16.3

[RFC PATCH v3 10/10] ima/ima_mmap: Rewrite to new library

[Petr Vorel]

Filename passed as getopt parameter.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 .../kernel/security/integrity/ima/src/ima_mmap.c   | 75 +++++++++++-----------
 .../security/integrity/ima/tests/ima_violations.sh |  2 +-
 2 files changed, 39 insertions(+), 38 deletions(-)

diff --git a/testcases/kernel/security/integrity/ima/src/ima_mmap.c b/testcases/kernel/security/integrity/ima/src/ima_mmap.c
index 9045e79a0..5bc688bd4 100644
--- a/testcases/kernel/security/integrity/ima/src/ima_mmap.c
+++ b/testcases/kernel/security/integrity/ima/src/ima_mmap.c
@@ -14,48 +14,49 @@
  * Open and mmap a file and sleep. Another process will open the
  * mmapped file in read mode, resulting in a open_writer violation.
  */
-#include <stdio.h>
-#include <string.h>
-#include <unistd.h>
-#include <sys/stat.h>
-#include <sys/mman.h>
-#include <fcntl.h>
-#include "test.h"
 
-char *TCID = "ima_mmap";
-int TST_TOTAL = 1;
+#include "tst_test.h"
 
 #define SLEEP_AFTER_CLOSE 3
+#define MMAPSIZE 1024
 
-int main(int argc, char *argv[])
+static char *filename;
+static void *file;
+static int fd;
+
+static struct tst_option options[] = {
+	{"f:", &filename,
+	 "-f file  File to mmap"},
+	{NULL, NULL, NULL}
+};
+
+static void cleanup(void)
+{
+	if (file)
+		SAFE_MUNMAP(file, MMAPSIZE);
+
+	if (fd > 0)
+		SAFE_CLOSE(fd);
+}
+
+static void run(void)
 {
-	int fd;
-	void *file;
-	char *filename;
-
-	if (argc != 2)
-		printf("%s: filename\n", argv[1]);
-	filename = argv[1];
-
-	fd = open(filename, O_CREAT | O_RDWR, S_IRWXU);
-	if (fd < 0) {
-		perror("open");
-		return (-1);
-	}
-
-	file = mmap(NULL, 1024, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
-	if (file == (void *)-1) {
-		perror("mmap");
-		return (-1);
-	}
-	close(fd);
-
-	tst_resm(TINFO, "sleep %ds", SLEEP_AFTER_CLOSE);
+	if (!filename)
+		tst_brk(TBROK, "Usage: %s -f filename", TCID);
+
+	fd = SAFE_OPEN(filename, O_CREAT | O_RDWR, S_IRWXU);
+
+	file = SAFE_MMAP(NULL, MMAPSIZE, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
+	SAFE_CLOSE(fd);
+
+	tst_res(TINFO, "sleep %ds", SLEEP_AFTER_CLOSE);
 	sleep(SLEEP_AFTER_CLOSE);
 
-	if (munmap(file, 1024) < 0) {
-		perror("unmap");
-		return (-1);
-	}
-	tst_exit();
+	tst_res(TPASS, "test completed");
 }
+
+static struct tst_test test = {
+	.options = options,
+	.test_all = run,
+	.cleanup = cleanup,
+};
diff --git a/testcases/kernel/security/integrity/ima/tests/ima_violations.sh b/testcases/kernel/security/integrity/ima/tests/ima_violations.sh
index 8742f4593..f3f40d455 100755
--- a/testcases/kernel/security/integrity/ima/tests/ima_violations.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_violations.sh
@@ -142,7 +142,7 @@ test3()
 
 	echo 'testing testing' > $FILE
 
-	ima_mmap $FILE &
+	ima_mmap -f $FILE &
 	# wait for violations appear in logs
 	tst_sleep 1s
 
-- 
2.16.3

Re: [LTP] [RFC PATCH v3 10/10] ima/ima_mmap: Rewrite to new library

[Cyril Hrubis]

Hi!
> -	ima_mmap $FILE &
> +	ima_mmap -f $FILE &
>  	# wait for violations appear in logs
>  	tst_sleep 1s

Whenever we wait for logs we should poll for them.

But same as the sleep in previous case, let's get these tests fixed
first, then we can improve on the performance and robustness.

-- 
Cyril Hrubis
chrubis@suse.cz

Re: [RFC PATCH v3 00/10] Rewrite tests into new API + fixes

[Mimi Zohar]

On Thu, 2018-04-19 at 21:54 +0200, Petr Vorel wrote:
> Hi,
> 
> changes v2->v3:
> * Fixed some of errors caused by test order.
> 
> * ima_boot_aggregate
>   - max event size is now 1MB according to spec
> 
> * ima_mmap
>   - reduce sleep + log it
>   - rewritten into new API
> 
> * ima_measurements.sh
>   - don't require iversion for kernel >= 4.16
>   - avoid using tmpfs

This is working nicely!

> 
> * ima_policy.sh
>   - improved detection of policy writability
>   - merge test2 and test3
> 
> * ima_violations.sh
>   - avoid using tmpfs
>   - improved grepping logs (no sleep is needed)
> 
> * ima_tpm.sh
>   - Improve error messages
> 
> TODO:
> * fix problems with violations tests (see patch 02/10).
> * detect whether policy must be signed (currently tests assume the
> policy does not need to be signed):
> https://lists.linux.it/pipermail/ltp/2018-April/007702.html
> http://lists.linux.it/pipermail/ltp/2018-January/006970.html


test: cmdline="ima_policy.sh"
contacts=""
analysis=exit
<<<test_output>>>
ima_policy 1 TINFO: verify that invalid policy isn't loaded
ima_policy 1 TPASS: didn't load invalid policy
ima_policy 2 TINFO: verify that policy file is not opened concurrently
and able to loaded multiple times
ima_policy 2 TFAIL: problem with loading policy (policy should be able
to load multiple times)

For now, could we change "problem with loading policy (policy should
be able to load multiple times)" to say, "problem loading or extending
policy (may require policy to be signed)"?

I'm also seeing, 

test: ima_tpm
<<<test_output>>>
ima_tpm 1 TINFO: verify boot aggregate
ima_tpm 1 TPASS: bios aggregate matches IMA boot aggregate
ima_tpm 2 TINFO: verify PCR values
ima_tpm 2 TINFO: evmctl version: evmctl 1.0
ima_tpm 2 TINFO: new PCRS path, evmctl >= 1.1 required
ima_tpm 2 TINFO: verify PCR (Process Control Register)
ima_tpm 2 TFAIL: failed to get PCR-10
ima_tpm 2 TPASS: aggregate PCR value matches real PCR value

It's unclear how the script could fail to get PCR-10, but pass the
following test.

Mimi

Re: [RFC PATCH v3 00/10] Rewrite tests into new API + fixes

[Petr Vorel]

Hi,

> > * ima_measurements.sh
> >   - don't require iversion for kernel >= 4.16
> >   - avoid using tmpfs

> This is working nicely!
:).


...
> test: cmdline="ima_policy.sh"
> contacts=""
> analysis=exit
> <<<test_output>>>
> ima_policy 1 TINFO: verify that invalid policy isn't loaded
> ima_policy 1 TPASS: didn't load invalid policy
> ima_policy 2 TINFO: verify that policy file is not opened concurrently
> and able to loaded multiple times
> ima_policy 2 TFAIL: problem with loading policy (policy should be able
> to load multiple times)

> For now, could we change "problem with loading policy (policy should
> be able to load multiple times)" to say, "problem loading or extending
> policy (may require policy to be signed)"?
Sure, thanks!


> I'm also seeing, 

> test: ima_tpm
> <<<test_output>>>
> ima_tpm 1 TINFO: verify boot aggregate
> ima_tpm 1 TPASS: bios aggregate matches IMA boot aggregate
> ima_tpm 2 TINFO: verify PCR values
> ima_tpm 2 TINFO: evmctl version: evmctl 1.0
> ima_tpm 2 TINFO: new PCRS path, evmctl >= 1.1 required
> ima_tpm 2 TINFO: verify PCR (Process Control Register)
> ima_tpm 2 TFAIL: failed to get PCR-10
> ima_tpm 2 TPASS: aggregate PCR value matches real PCR value

> It's unclear how the script could fail to get PCR-10, but pass the
> following test.
Thanks, fixed (wrong return).

> Mimi


Kind regards,
Petr

Re: [LTP] [RFC PATCH v3 00/10] Rewrite tests into new API + fixes

[Petr Vorel]

Hi,

> changes v2->v3:
> * Fixed some of errors caused by test order.

> * ima_boot_aggregate
>   - max event size is now 1MB according to spec

> * ima_mmap
>   - reduce sleep + log it
>   - rewritten into new API

> * ima_measurements.sh
>   - don't require iversion for kernel >= 4.16
>   - avoid using tmpfs

> * ima_policy.sh
>   - improved detection of policy writability
>   - merge test2 and test3

> * ima_violations.sh
>   - avoid using tmpfs
>   - improved grepping logs (no sleep is needed)

> * ima_tpm.sh
>   - Improve error messages

> TODO:
> * fix problems with violations tests (see patch 02/10).
> * detect whether policy must be signed (currently tests assume the
> policy does not need to be signed):
> https://lists.linux.it/pipermail/ltp/2018-April/007702.html
> http://lists.linux.it/pipermail/ltp/2018-January/006970.html

Merged. See diff against v3, if interested.
Thanks a lot Mimi for your comments, tips and review.

TODO:

* detect whether policy must be signed (currently tests assume the
policy does not need to be signed):
https://lists.linux.it/pipermail/ltp/2018-April/007702.html
http://lists.linux.it/pipermail/ltp/2018-January/006970.html

* ima_violations are failing on logging into /var/log/messages (without auditd):

tst_device.c:83: INFO: Found free device '/dev/loop0'
ima_violations 1 TINFO: /proc/cmdline: BOOT_IMAGE=/vmlinuz-4.10.0-rc6-kaiser root=/dev/mapp             er/debian--testing--vg-root ro quiet ima_policy=secure_boot
ima_violations 1 TINFO: IMA kernel config
ima_violations 1 TINFO: CONFIG_IMA=y
ima_violations 1 TINFO: CONFIG_IMA_MEASURE_PCR_IDX=10
ima_violations 1 TINFO: CONFIG_IMA_LSM_RULES=y
ima_violations 1 TINFO: CONFIG_IMA_NG_TEMPLATE=y
ima_violations 1 TINFO: CONFIG_IMA_DEFAULT_TEMPLATE="ima-ng"
ima_violations 1 TINFO: CONFIG_IMA_DEFAULT_HASH_SHA1=y
ima_violations 1 TINFO: CONFIG_IMA_DEFAULT_HASH="sha1"
ima_violations 1 TINFO: CONFIG_IMA_WRITE_POLICY=y
ima_violations 1 TINFO: CONFIG_IMA_READ_POLICY=y
ima_violations 1 TINFO: CONFIG_IMA_APPRAISE=y
ima_violations 1 TINFO: CONFIG_IMA_TRUSTED_KEYRING=y
ima_violations 1 TINFO: CONFIG_IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY=y
ima_violations 1 TINFO: CONFIG_IMA_BLACKLIST_KEYRING=y
ima_violations 1 TINFO: $TMPDIR is on tmpfs => run on loop device
ima_violations 1 TINFO: Formatting /dev/loop0 with ext3 extra opts=''
ima_violations 1 TINFO: using log /var/log/messages
ima_violations 1 TINFO: verify open writers violation
ima_violations 1 TINFO: open_writers not found in /var/log/messages (1/3 attempt)...
ima_violations 1 TINFO: open_writers not found in /var/log/messages (2/3 attempt)...
ima_violations 1 TINFO: open_writers not found in /var/log/messages (3/3 attempt)...
ima_violations 1 TFAIL: open_writers not found in /var/log/messages
ima_violations 2 TINFO: verify ToMToU violation
ima_violations 2 TINFO: ToMToU not found in /var/log/messages (1/3 attempt)...
ima_violations 2 TINFO: ToMToU not found in /var/log/messages (2/3 attempt)...
ima_violations 2 TINFO: ToMToU not found in /var/log/messages (3/3 attempt)...
ima_violations 2 TFAIL: ToMToU not found in /var/log/messages
...
This is due previous test ima_policy running (when there is not
possible write to policy, e.g. second run of the testsuites on CONFIG_IMA_WRITE_POLICY=n
it's ok)
I wonder if we should just TCONF when logging into /var/log/messages with combination of
policy being writable (or TCONF when logging into /var/log/messages in any case).


* Check whether current policy has tbc (i.e. presence of "ima_tcb" or "tcb" being part of ima_policy in
/proc/cmdline) [1]. I wonder if we should TCONF all tests without tcb (some tests are
working

* Getting record with old kernels (tested on both deprecated ima_tbc and ima_policy=tcb):
ima_measurements 1 TINFO: /proc/cmdline: BOOT_IMAGE=/vmlinuz-3.10.0-693.2.2.el7.x86_64 root=/dev/mapper/centos-root ro crashkernel=auto rd.lvm.lv=centos/root rd.lvm.lv=centos/swap rhgb quiet ima_tbc
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_AUDIT=y
ima_measurements 1 TINFO: CONFIG_IMA_LSM_RULES=y
ima_measurements 1 TINFO: CONFIG_IMA_APPRAISE=y
ima_measurements 1 TINFO: CONFIG_IMA_TRUSTED_KEYRING=y
ima_measurements 1 TINFO: verify adding record to the IMA measurement list
ima_measurements 1 TFAIL: cannot find measurement for '/tmp/netpan-1253/LTP_ima_measurements.P2uyOze2J4/test.txt'
awk: cmd. line:1: (FILENAME=- FNR=1) fatal: attempt to access field -1
ima_measurements 1 TINFO: computing hash for sha1 digest
ima_measurements 1 TFAIL: hash not found
ima_measurements 2 TINFO: verify updating record in the IMA measurement list
ima_measurements 2 TCONF: XFS Filesystem >= V5 required for iversion support
ima_measurements 3 TINFO: verify not measuring user files
ima_measurements 3 TPASS: grep /tmp/netpan-1253/LTP_ima_measurements.P2uyOze2J4/user/test.txt /sys/kernel/security/ima/ascii_runtime_measurements failed as expected

Not sure if this is caused by different IMA behavior in old kernels or due configuration.

Kind regards,
Petr

[1] https://lists.linux.it/pipermail/ltp/2018-April/007906.html


Diff against v3:
diff --git runtest/ima runtest/ima
index e7824a62a..bcae16bb7 100644
--- runtest/ima
+++ runtest/ima
@@ -1,5 +1,5 @@
 #DESCRIPTION:Integrity Measurement Architecture (IMA)
-ima_violations ima_violations.sh
-ima_policy ima_policy.sh
 ima_measurements ima_measurements.sh
+ima_policy ima_policy.sh
 ima_tpm ima_tpm.sh
+ima_violations ima_violations.sh
diff --git testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
index 862cc07ba..f6e7be041 100644
--- testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
+++ testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
@@ -81,7 +81,7 @@ int main(int argc, char *argv[])
 	for (i = 0; i < NUM_PCRS; i++)
 		memset(&pcr[i].digest, 0, SHA_DIGEST_LENGTH);
 
-	event.data = (char *) malloc(MAX_EVENT_DATA_SIZE);
+	event.data = malloc(MAX_EVENT_DATA_SIZE);
 	if (!event.data) {
 		printf("Cannot allocate memory\n");
 		return 1;
diff --git testcases/kernel/security/integrity/ima/tests/ima_policy.sh testcases/kernel/security/integrity/ima/tests/ima_policy.sh
index 1c4a0b922..64aa8cb7a 100755
--- testcases/kernel/security/integrity/ima/tests/ima_policy.sh
+++ testcases/kernel/security/integrity/ima/tests/ima_policy.sh
@@ -95,7 +95,7 @@ test2()
 	elif [ $rc1 -eq 0 ] || [ $rc2 -eq 0 ]; then
 		tst_res TPASS "policy was loaded just by one process and able to loaded multiple times"
 	else
-		tst_res TFAIL "problem with loading policy (policy should be able to load multiple times)"
+		tst_res TFAIL "problem loading or extending policy (may require policy to be signed)"
 	fi
 }
 
diff --git testcases/kernel/security/integrity/ima/tests/ima_setup.sh testcases/kernel/security/integrity/ima/tests/ima_setup.sh
index 03851167f..8ea7aec18 100644
--- testcases/kernel/security/integrity/ima/tests/ima_setup.sh
+++ testcases/kernel/security/integrity/ima/tests/ima_setup.sh
@@ -64,6 +64,21 @@ mount_loop_device()
 	cd mntpoint
 }
 
+print_ima_config()
+{
+	local config="/boot/config-$(uname -r)"
+	local i
+
+	tst_res TINFO "/proc/cmdline: $(cat /proc/cmdline)"
+
+	if [ -r "$config" ]; then
+		tst_res TINFO "IMA kernel config:"
+		for i in $(grep ^CONFIG_IMA $config); do
+			tst_res TINFO "$i"
+		done
+	fi
+}
+
 ima_setup()
 {
 	SECURITYFS="$(mount_helper securityfs $SYSFS/kernel/security)"
@@ -73,14 +88,14 @@ ima_setup()
 	ASCII_MEASUREMENTS="$IMA_DIR/ascii_runtime_measurements"
 	BINARY_MEASUREMENTS="$IMA_DIR/binary_runtime_measurements"
 
+	print_ima_config
+
 	if [ "$TST_NEEDS_DEVICE" = 1 ]; then
 		tst_res TINFO "\$TMPDIR is on tmpfs => run on loop device"
 		mount_loop_device
 	fi
 
-	if [ -n "$TST_SETUP_CALLER" ]; then
-		$TST_SETUP_CALLER
-	fi
+	[ -n "$TST_SETUP_CALLER" ] && $TST_SETUP_CALLER
 }
 
 ima_cleanup()
diff --git testcases/kernel/security/integrity/ima/tests/ima_tpm.sh testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
index 0124c338f..0ffc3c022 100755
--- testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
+++ testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
@@ -69,7 +69,7 @@ validate_pcr()
 		grep 'HW PCR-10:' | awk '{print $3}')"
 	if [ -z "$aggregate_pcr" ]; then
 		tst_res TFAIL "failed to get PCR-10"
-		return
+		return 1
 	fi
 
 	while read line; do

Re: [LTP] [RFC PATCH v3 00/10] Rewrite tests into new API + fixes

[Mimi Zohar]

On Fri, 2018-04-27 at 11:51 +0200, Petr Vorel wrote:
[...]
>  ima_cleanup()
> diff --git testcases/kernel/security/integrity/ima/tests/ima_tpm.sh testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
> index 0124c338f..0ffc3c022 100755
> --- testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
> +++ testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
> @@ -69,7 +69,7 @@ validate_pcr()
>  		grep 'HW PCR-10:' | awk '{print $3}')"
>  	if [ -z "$aggregate_pcr" ]; then
>  		tst_res TFAIL "failed to get PCR-10"
> -		return
> +		return 1
>  	fi
> 
>  	while read line; do
> 

        aggregate_pcr="$(evmctl -v ima_measurement
$BINARY_MEASUREMENTS 2>&1 | \
                grep 'HW PCR-10:' | awk '{print $3}')"

This works properly with the "ima-ng" template and even the "ima-sig"
template, without any signatures or keys.  With the "ima-sig" there
will be some informational/warning messages.  Even with the warnings,
we can still validate the measurement list PCR value.

The last two records will contain the calculated aggregate PCR value
and the real HW PCR value. 

example 1: evmctl without any keys
[...]
Failed to open keyfile: /etc/keys/x509_evm.der
PCRAgg 10: 2d1f635489a5b82fafde1ed48cfe67eabf6cba7b
HW PCR-10: 2d1f635489a5b82fafde1ed48cfe67eabf6cba7b

example 2: evmctl missing some keys
key 1: 6e6c1046 /etc/keys/ima/<additional key>
[...]
/usr/lib64/evolution/plugins/liborg-gnome-email-custom-header.so: RSA_public_decrypt() failed: -1
/usr/lib64/evolution/plugins/liborg-gnome-external-editor.so: RSA_public_decrypt() failed: -1
PCRAgg 10: 2d1f635489a5b82fafde1ed48cfe67eabf6cba7b
HW PCR-10: 2d1f635489a5b82fafde1ed48cfe67eabf6cba7b

example 3: evmctl with all keys ("-k" option) 
key 1: 6e6c1046 /etc/keys/ima/<distro key>
key 2: c4e2426e /etc/keys/ima/<additional key>
PCRAgg 10: 2d1f635489a5b82fafde1ed48cfe67eabf6cba7b
HW PCR-10: 2d1f635489a5b82fafde1ed48cfe67eabf6cba7b

Mimi

Re: [LTP] [RFC PATCH v3 00/10] Rewrite tests into new API + fixes

[Mimi Zohar]

On Fri, 2018-04-27 at 11:51 +0200, Petr Vorel wrote:
> Hi,
> 
> > changes v2->v3:
> > * Fixed some of errors caused by test order.
> 
> > * ima_boot_aggregate
> >   - max event size is now 1MB according to spec
> 
> > * ima_mmap
> >   - reduce sleep + log it
> >   - rewritten into new API
> 
> > * ima_measurements.sh
> >   - don't require iversion for kernel >= 4.16
> >   - avoid using tmpfs
> 
> > * ima_policy.sh
> >   - improved detection of policy writability
> >   - merge test2 and test3
> 
> > * ima_violations.sh
> >   - avoid using tmpfs
> >   - improved grepping logs (no sleep is needed)
> 
> > * ima_tpm.sh
> >   - Improve error messages
> 
> > TODO:
> > * fix problems with violations tests (see patch 02/10).
> > * detect whether policy must be signed (currently tests assume the
> > policy does not need to be signed):
> > https://lists.linux.it/pipermail/ltp/2018-April/007702.html
> > http://lists.linux.it/pipermail/ltp/2018-January/006970.html
> 
> Merged. See diff against v3, if interested.
> Thanks a lot Mimi for your comments, tips and review.

Thank you for working on this and cleaning it up!

> 
> TODO:
> 
> * detect whether policy must be signed (currently tests assume the
> policy does not need to be signed):
> https://lists.linux.it/pipermail/ltp/2018-April/007702.html
> http://lists.linux.it/pipermail/ltp/2018-January/006970.html
> 
> * ima_violations are failing on logging into /var/log/messages (without auditd):
> 
> tst_device.c:83: INFO: Found free device '/dev/loop0'
> ima_violations 1 TINFO: /proc/cmdline: BOOT_IMAGE=/vmlinuz-4.10.0-rc6-kaiser root=/dev/mapp             er/debian--testing--vg-root ro quiet ima_policy=secure_boot
> ima_violations 1 TINFO: IMA kernel config
> ima_violations 1 TINFO: CONFIG_IMA=y
> ima_violations 1 TINFO: CONFIG_IMA_MEASURE_PCR_IDX=10
> ima_violations 1 TINFO: CONFIG_IMA_LSM_RULES=y
> ima_violations 1 TINFO: CONFIG_IMA_NG_TEMPLATE=y
> ima_violations 1 TINFO: CONFIG_IMA_DEFAULT_TEMPLATE="ima-ng"
> ima_violations 1 TINFO: CONFIG_IMA_DEFAULT_HASH_SHA1=y
> ima_violations 1 TINFO: CONFIG_IMA_DEFAULT_HASH="sha1"
> ima_violations 1 TINFO: CONFIG_IMA_WRITE_POLICY=y
> ima_violations 1 TINFO: CONFIG_IMA_READ_POLICY=y
> ima_violations 1 TINFO: CONFIG_IMA_APPRAISE=y
> ima_violations 1 TINFO: CONFIG_IMA_TRUSTED_KEYRING=y
> ima_violations 1 TINFO: CONFIG_IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY=y
> ima_violations 1 TINFO: CONFIG_IMA_BLACKLIST_KEYRING=y
> ima_violations 1 TINFO: $TMPDIR is on tmpfs => run on loop device
> ima_violations 1 TINFO: Formatting /dev/loop0 with ext3 extra opts=''
> ima_violations 1 TINFO: using log /var/log/messages
> ima_violations 1 TINFO: verify open writers violation
> ima_violations 1 TINFO: open_writers not found in /var/log/messages (1/3 attempt)...
> ima_violations 1 TINFO: open_writers not found in /var/log/messages (2/3 attempt)...
> ima_violations 1 TINFO: open_writers not found in /var/log/messages (3/3 attempt)...
> ima_violations 1 TFAIL: open_writers not found in /var/log/messages
> ima_violations 2 TINFO: verify ToMToU violation
> ima_violations 2 TINFO: ToMToU not found in /var/log/messages (1/3 attempt)...
> ima_violations 2 TINFO: ToMToU not found in /var/log/messages (2/3 attempt)...
> ima_violations 2 TINFO: ToMToU not found in /var/log/messages (3/3 attempt)...
> ima_violations 2 TFAIL: ToMToU not found in /var/log/messages
> ...
> This is due previous test ima_policy running (when there is not
> possible write to policy, e.g. second run of the testsuites on CONFIG_IMA_WRITE_POLICY=n
> it's ok)

If there isn't any policy, then these results would be expected.

> I wonder if we should just TCONF when logging into /var/log/messages with combination of
> policy being writable (or TCONF when logging into /var/log/messages in any case).
> 
> * Check whether current policy has tbc (i.e. presence of "ima_tcb" or "tcb" being part of ima_policy in
> /proc/cmdline) [1]. I wonder if we should TCONF all tests without tcb (some tests are
> working

For the case of no policy, you could still run the boot-aggregate
test.  I'm not sure about any of the other tests.

Even if the system was booted with either of the "tcb" policies, it
could still have been replaced with a custom policy.  If we're able to
cat the policy, we could verify that the loaded policy includes the
"tcb" policy and emit a TCONF warning message for non tcb policies.

For now, perhaps add a general message indicating that the tests
assume a tcb policy. 

> 
> * Getting record with old kernels (tested on both deprecated ima_tbc and ima_policy=tcb):

^ima_tcb  

> ima_measurements 1 TINFO: /proc/cmdline: BOOT_IMAGE=/vmlinuz-3.10.0-693.2.2.el7.x86_64 root=/dev/mapper/centos-root ro crashkernel=auto rd.lvm.lv=centos/root rd.lvm.lv=centos/swap rhgb quiet ima_tbc
> 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_AUDIT=y
> ima_measurements 1 TINFO: CONFIG_IMA_LSM_RULES=y
> ima_measurements 1 TINFO: CONFIG_IMA_APPRAISE=y
> ima_measurements 1 TINFO: CONFIG_IMA_TRUSTED_KEYRING=y
> ima_measurements 1 TINFO: verify adding record to the IMA measurement list
> ima_measurements 1 TFAIL: cannot find measurement for '/tmp/netpan-1253/LTP_ima_measurements.P2uyOze2J4/test.txt'
> awk: cmd. line:1: (FILENAME=- FNR=1) fatal: attempt to access field -1
> ima_measurements 1 TINFO: computing hash for sha1 digest
> ima_measurements 1 TFAIL: hash not found
> ima_measurements 2 TINFO: verify updating record in the IMA measurement list
> ima_measurements 2 TCONF: XFS Filesystem >= V5 required for iversion support
> ima_measurements 3 TINFO: verify not measuring user files
> ima_measurements 3 TPASS: grep /tmp/netpan-1253/LTP_ima_measurements.P2uyOze2J4/user/test.txt /sys/kernel/security/ima/ascii_runtime_measurements failed as expected
> 
> Not sure if this is caused by different IMA behavior in old kernels or due configuration.

Maybe just a typo - ima_tcb, not ima_tbc.

Mimi

Re: [LTP] [RFC PATCH v3 00/10] Rewrite tests into new API + fixes

[Petr Vorel]

Hi Mimi,

> > * ima_violations are failing on logging into /var/log/messages (without auditd):

> > tst_device.c:83: INFO: Found free device '/dev/loop0'
> > ima_violations 1 TINFO: /proc/cmdline: BOOT_IMAGE=/vmlinuz-4.10.0-rc6-kaiser root=/dev/mapp             er/debian--testing--vg-root ro quiet ima_policy=secure_boot
> > ima_violations 1 TINFO: IMA kernel config
> > ima_violations 1 TINFO: CONFIG_IMA=y
> > ima_violations 1 TINFO: CONFIG_IMA_MEASURE_PCR_IDX=10
> > ima_violations 1 TINFO: CONFIG_IMA_LSM_RULES=y
> > ima_violations 1 TINFO: CONFIG_IMA_NG_TEMPLATE=y
> > ima_violations 1 TINFO: CONFIG_IMA_DEFAULT_TEMPLATE="ima-ng"
> > ima_violations 1 TINFO: CONFIG_IMA_DEFAULT_HASH_SHA1=y
> > ima_violations 1 TINFO: CONFIG_IMA_DEFAULT_HASH="sha1"
> > ima_violations 1 TINFO: CONFIG_IMA_WRITE_POLICY=y
> > ima_violations 1 TINFO: CONFIG_IMA_READ_POLICY=y
> > ima_violations 1 TINFO: CONFIG_IMA_APPRAISE=y
> > ima_violations 1 TINFO: CONFIG_IMA_TRUSTED_KEYRING=y
> > ima_violations 1 TINFO: CONFIG_IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY=y
> > ima_violations 1 TINFO: CONFIG_IMA_BLACKLIST_KEYRING=y
> > ima_violations 1 TINFO: $TMPDIR is on tmpfs => run on loop device
> > ima_violations 1 TINFO: Formatting /dev/loop0 with ext3 extra opts=''
> > ima_violations 1 TINFO: using log /var/log/messages
> > ima_violations 1 TINFO: verify open writers violation
> > ima_violations 1 TINFO: open_writers not found in /var/log/messages (1/3 attempt)...
> > ima_violations 1 TINFO: open_writers not found in /var/log/messages (2/3 attempt)...
> > ima_violations 1 TINFO: open_writers not found in /var/log/messages (3/3 attempt)...
> > ima_violations 1 TFAIL: open_writers not found in /var/log/messages
> > ima_violations 2 TINFO: verify ToMToU violation
> > ima_violations 2 TINFO: ToMToU not found in /var/log/messages (1/3 attempt)...
> > ima_violations 2 TINFO: ToMToU not found in /var/log/messages (2/3 attempt)...
> > ima_violations 2 TINFO: ToMToU not found in /var/log/messages (3/3 attempt)...
> > ima_violations 2 TFAIL: ToMToU not found in /var/log/messages
> > ...
> > This is due previous test ima_policy running (when there is not
> > possible write to policy, e.g. second run of the testsuites on CONFIG_IMA_WRITE_POLICY=n
> > it's ok)

> If there isn't any policy, then these results would be expected.
No, ima_violations with /var/log/messages are failing even with tcb policy loaded (on kernels >= 4.x).

> > I wonder if we should just TCONF when logging into /var/log/messages with combination of
> > policy being writable (or TCONF when logging into /var/log/messages in any case).

> > * Check whether current policy has tbc (i.e. presence of "ima_tcb" or "tcb" being part of ima_policy in
> > /proc/cmdline) [1]. I wonder if we should TCONF all tests without tcb (some tests are
> > working

> For the case of no policy, you could still run the boot-aggregate
> test.  I'm not sure about any of the other tests.
I'll check which ones are working and not issue TCONF for them.

> Even if the system was booted with either of the "tcb" policies, it
> could still have been replaced with a custom policy.  If we're able to
> cat the policy, we could verify that the loaded policy includes the
> "tcb" policy and emit a TCONF warning message for non tcb policies.
I understand you as checking /sys/kernel/security/ima/policy (assumes
CONFIG_IMA_READ_POLICY) to have content defined in kernel ima_rule_entry
default_measurement_rules[] (from ima_policy.c from kernel).

> For now, perhaps add a general message indicating that the tests
> assume a tcb policy. 
Make sense, I'll add it now.


> > * Getting record with old kernels (tested on both deprecated ima_tbc and ima_policy=tcb):

> ^ima_tcb  

> > ima_measurements 1 TINFO: /proc/cmdline: BOOT_IMAGE=/vmlinuz-3.10.0-693.2.2.el7.x86_64 root=/dev/mapper/centos-root ro crashkernel=auto rd.lvm.lv=centos/root rd.lvm.lv=centos/swap rhgb quiet ima_tbc
> > 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_AUDIT=y
> > ima_measurements 1 TINFO: CONFIG_IMA_LSM_RULES=y
> > ima_measurements 1 TINFO: CONFIG_IMA_APPRAISE=y
> > ima_measurements 1 TINFO: CONFIG_IMA_TRUSTED_KEYRING=y
> > ima_measurements 1 TINFO: verify adding record to the IMA measurement list
> > ima_measurements 1 TFAIL: cannot find measurement for '/tmp/netpan-1253/LTP_ima_measurements.P2uyOze2J4/test.txt'
> > awk: cmd. line:1: (FILENAME=- FNR=1) fatal: attempt to access field -1
> > ima_measurements 1 TINFO: computing hash for sha1 digest
> > ima_measurements 1 TFAIL: hash not found
> > ima_measurements 2 TINFO: verify updating record in the IMA measurement list
> > ima_measurements 2 TCONF: XFS Filesystem >= V5 required for iversion support
> > ima_measurements 3 TINFO: verify not measuring user files
> > ima_measurements 3 TPASS: grep /tmp/netpan-1253/LTP_ima_measurements.P2uyOze2J4/user/test.txt /sys/kernel/security/ima/ascii_runtime_measurements failed as expected

> > Not sure if this is caused by different IMA behavior in old kernels or due configuration.

> Maybe just a typo - ima_tcb, not ima_tbc.
Yes, that was the reason (silly mistake). On older kernels 3.x only ima_tbc (I'll check
kernel versions and let user to know correct variable in TCONF).

> Mimi