All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH v2 0/4] Rewrite tests into new API + fixes
@ 2018-03-14 15:57 ` Petr Vorel
  0 siblings, 0 replies; 31+ messages in thread
From: Petr Vorel @ 2018-03-14 15:57 UTC (permalink / raw)
  To: ltp; +Cc: Petr Vorel, Mimi Zohar, linux-integrity

Hi,

this is a second attempt to rewrite IMA tests.
Comments and fixes are welcome.

Changes v1->v2:
* ima_measurements.sh: add support for "ima-ng" and "ima-sig" IMA
  measurement templates
* ima_measurements.sh: add support for most of hash algorithms is
  defined in include/uapi/linux/hash_info.h
* fix ima_boot_aggregate ("ima/ima_boot_aggregate: Increase MAX_EVENT_SIZE to 8k")
* ima_tpm.sh: fixes of TPM test ("ima/tpm: Various fixes")
* ima_measurements.sh: drop ima_measure and use evmctl (external dependency) instead
* ima_measurements.sh: check XFS version for iversion support

TODO
* ima_measurements.sh: Add support for ima_template_fmt kernel parameter.

* ima_policy.sh: Detect if the policy must be signed [1] (IMA_WRITE_POLICY or
"secure_boot" kernel parameter).
@Mimi: What is a best approach in case policy must be signed? measure.policy
and measure.policy-invalid files are not signed should we skip all tests in
ima_policy.sh with something like "Not supported when policy must be signed"?
Or running them both and expect them to fail as they're not signed?
As that's how I understand your related commit in kernel
(19f8a84713ed "ima: measure and appraise the IMA policy itself").
BTW load_policy() use old approach catting the content into sysfs policy file.
Maybe it'd be good to echo policy filename into sysfs policy file for kernel >
4.6 (feature added in 7429b092811f "ima: load policy using path").

* ima_measurement.sh,ima_violations.sh: Avoid using tmpfs filesystem [1]. You
suggested using RAM block device. Would it be ok to use filesystem created on
loop device (/dev/loop0)? Or even create image file in $TMPDIR (mostly
/tmp, which can be tmpfs) and use it as a loop device?

To be honnest, I'm not sure if I addressed your comment [2]:
These tests are for the IMA-measurement aspect only, not IMA-
appraisal.  Adding measurements to the measurement list won't cause
the system to stop working, unless keys are sealed to a particular TPM
PCR value.  Nobody is or should be sealing keys to PCR-10, since the
ordering of the measurements is non deterministic.
As we add IMA-appraisal tests requiring files to be signed, things
will fail if either the public key isn't on the IMA keyring or the
file isn't properly signed.  For this reason, limiting file IMA-
appraisal tests to a particular filesystem simplifies testing.

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


Kind regards,
Petr


Petr Vorel (4):
  security/ima: Rewrite tests into new API + fixes
  security/ima: Run measurements after policy
  ima/ima_boot_aggregate: Increase MAX_EVENT_SIZE to 8k
  ima/tpm: Various fixes

 runtest/ima                                        |   8 +-
 testcases/kernel/security/integrity/.gitignore     |   1 -
 .../integrity/ima/src/ima_boot_aggregate.c         |   2 +-
 .../security/integrity/ima/src/ima_measure.c       | 219 -------------------
 .../integrity/ima/tests/ima_measurements.sh        | 239 +++++++++++----------
 .../security/integrity/ima/tests/ima_policy.sh     | 148 ++++++-------
 .../security/integrity/ima/tests/ima_setup.sh      | 110 ++++------
 .../kernel/security/integrity/ima/tests/ima_tpm.sh | 160 ++++++--------
 .../security/integrity/ima/tests/ima_violations.sh | 217 +++++++++----------
 9 files changed, 417 insertions(+), 687 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.2

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [LTP] [RFC PATCH v2 0/4] Rewrite tests into new API + fixes
@ 2018-03-14 15:57 ` Petr Vorel
  0 siblings, 0 replies; 31+ messages in thread
From: Petr Vorel @ 2018-03-14 15:57 UTC (permalink / raw)
  To: ltp

Hi,

this is a second attempt to rewrite IMA tests.
Comments and fixes are welcome.

Changes v1->v2:
* ima_measurements.sh: add support for "ima-ng" and "ima-sig" IMA
  measurement templates
* ima_measurements.sh: add support for most of hash algorithms is
  defined in include/uapi/linux/hash_info.h
* fix ima_boot_aggregate ("ima/ima_boot_aggregate: Increase MAX_EVENT_SIZE to 8k")
* ima_tpm.sh: fixes of TPM test ("ima/tpm: Various fixes")
* ima_measurements.sh: drop ima_measure and use evmctl (external dependency) instead
* ima_measurements.sh: check XFS version for iversion support

TODO
* ima_measurements.sh: Add support for ima_template_fmt kernel parameter.

* ima_policy.sh: Detect if the policy must be signed [1] (IMA_WRITE_POLICY or
"secure_boot" kernel parameter).
@Mimi: What is a best approach in case policy must be signed? measure.policy
and measure.policy-invalid files are not signed should we skip all tests in
ima_policy.sh with something like "Not supported when policy must be signed"?
Or running them both and expect them to fail as they're not signed?
As that's how I understand your related commit in kernel
(19f8a84713ed "ima: measure and appraise the IMA policy itself").
BTW load_policy() use old approach catting the content into sysfs policy file.
Maybe it'd be good to echo policy filename into sysfs policy file for kernel >
4.6 (feature added in 7429b092811f "ima: load policy using path").

* ima_measurement.sh,ima_violations.sh: Avoid using tmpfs filesystem [1]. You
suggested using RAM block device. Would it be ok to use filesystem created on
loop device (/dev/loop0)? Or even create image file in $TMPDIR (mostly
/tmp, which can be tmpfs) and use it as a loop device?

To be honnest, I'm not sure if I addressed your comment [2]:
These tests are for the IMA-measurement aspect only, not IMA-
appraisal.  Adding measurements to the measurement list won't cause
the system to stop working, unless keys are sealed to a particular TPM
PCR value.  Nobody is or should be sealing keys to PCR-10, since the
ordering of the measurements is non deterministic.
As we add IMA-appraisal tests requiring files to be signed, things
will fail if either the public key isn't on the IMA keyring or the
file isn't properly signed.  For this reason, limiting file IMA-
appraisal tests to a particular filesystem simplifies testing.

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


Kind regards,
Petr


Petr Vorel (4):
  security/ima: Rewrite tests into new API + fixes
  security/ima: Run measurements after policy
  ima/ima_boot_aggregate: Increase MAX_EVENT_SIZE to 8k
  ima/tpm: Various fixes

 runtest/ima                                        |   8 +-
 testcases/kernel/security/integrity/.gitignore     |   1 -
 .../integrity/ima/src/ima_boot_aggregate.c         |   2 +-
 .../security/integrity/ima/src/ima_measure.c       | 219 -------------------
 .../integrity/ima/tests/ima_measurements.sh        | 239 +++++++++++----------
 .../security/integrity/ima/tests/ima_policy.sh     | 148 ++++++-------
 .../security/integrity/ima/tests/ima_setup.sh      | 110 ++++------
 .../kernel/security/integrity/ima/tests/ima_tpm.sh | 160 ++++++--------
 .../security/integrity/ima/tests/ima_violations.sh | 217 +++++++++----------
 9 files changed, 417 insertions(+), 687 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.2


^ permalink raw reply	[flat|nested] 31+ messages in thread

* [RFC PATCH v2 1/4] security/ima: Rewrite tests into new API + fixes
  2018-03-14 15:57 ` [LTP] " Petr Vorel
@ 2018-03-14 15:57   ` Petr Vorel
  -1 siblings, 0 replies; 31+ messages in thread
From: Petr Vorel @ 2018-03-14 15:57 UTC (permalink / raw)
  To: ltp; +Cc: Petr Vorel, Mimi Zohar, linux-integrity

* 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 occurance of tested file in
    /sys/kernel/security/ima/ascii_runtime_measurements
  - 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
  - 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) + add sleep when SUT uses
    /var/log/messages to prevent failure during validate

* 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

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
Changes v1->v2:
I increased sleep to 1s I still occasionally encounter errors on systems
logging into /var/log/messages:
ima_violations 2 TFAIL: ToMToU not found in /var/log/messages
---
 runtest/ima                                        |   8 +-
 .../integrity/ima/tests/ima_measurements.sh        | 239 +++++++++++----------
 .../security/integrity/ima/tests/ima_policy.sh     | 148 ++++++-------
 .../security/integrity/ima/tests/ima_setup.sh      | 110 ++++------
 .../kernel/security/integrity/ima/tests/ima_tpm.sh | 140 ++++++------
 .../security/integrity/ima/tests/ima_violations.sh | 217 +++++++++----------
 6 files changed, 409 insertions(+), 453 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..353e45b30 100755
--- a/testcases/kernel/security/integrity/ima/tests/ima_measurements.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_measurements.sh
@@ -1,139 +1,156 @@
 #!/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_TESTFUNC="test"
+TST_SETUP="init"
+TST_CNT=3
+TST_NEEDS_TMPDIR=1
+
+. ima_setup.sh
+
+DEFAULT_DIGEST_OLD_FORMAT="sha1"
+POLICY="$IMA_DIR/policy"
 
 init()
 {
-	tst_check_cmds sha1sum
+	tst_check_cmds awk
 
-	# verify using default policy
-	if [ ! -f "$IMA_DIR/policy" ]; then
-		tst_resm TINFO "not using default policy"
-	fi
+	[ -f "$POLICY" ] || tst_res TINFO "not using default policy"
+
+	TEST_FILE="$PWD/test.txt"
+	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
 
-	# Calculating the sha1sum of test.txt should add
-	# the new measurement to the measurement list
-	hash=$(sha1sum test.txt | sed 's/  -//')
+	line="$(grep $TEST_FILE $ASCII_MEASUREMENTS | tail -1)"
+	[ -n "$line" ] || tst_res TFAIL "cannot find measurement for '$TEST_FILE'"
 
-	# Check if the new measurement exists
-	cat /sys/kernel/security/ima/ascii_runtime_measurements > measurements
-	$(grep $hash measurements > /dev/null)
+	[ "$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)"
 
-	if [ $? -ne 0 ]; then
-		tst_resm TFAIL "Modified file not measured"
-		tst_resm TINFO "iversion not supported; or not mounted with iversion"
+	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 [ "$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()
+test1()
 {
-	# 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
+	tst_res TINFO "verify adding record to the IMA measurement list"
+	ROD echo "$(date) this is a test file" \> $TEST_FILE
+	ima_check
 }
 
-. ima_setup.sh
+test2()
+{
+	local device mount fs
+
+	tst_res TINFO "verify updating record in the IMA measurement list"
+
+	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
+		fi
+		;;
+	xfs)
+		if dmesg | grep -q "XFS.*Mounting V[1-4] Filesystem"; then
+			tst_res TCONF "XFS Filesystem >= V5 required for iversion support"
+			return
+		fi
+		;;
+	'')
+		tst_res TWARN "could not find mount info for device '$device'"
+		;;
+	esac
+
+	ROD echo "$(date) modified file" \> $TEST_FILE
+	ima_check
+}
+
+test3()
+{
+	local user="nobody"
+	local dir="$PWD/user"
+	local file="$dir/test.txt"
 
-setup
-TST_CLEANUP=cleanup
+	# Default policy does not measure user files
+	tst_res TINFO "verify not measuring user files"
 
-init
-test01
-test02
-test03
+	if ! id $user >/dev/null 2>/dev/null; then
+		tst_res TCONF "missing system user $user (wrong installation)"
+		return
+	fi
+	tst_check_cmds sudo
+
+	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..08e34c6f8 100755
--- a/testcases/kernel/security/integrity/ima/tests/ima_policy.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_policy.sh
@@ -1,127 +1,115 @@
 #!/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_TESTFUNC="test"
+TST_SETUP="init"
+TST_CNT=3
+
+. ima_setup.sh
 
 init()
 {
-	# 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 doesn't replace default policy"
+
+	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()
 {
+	tst_res TINFO "verify that policy file is opened sequentially and installs new policy"
+
+	local p1 p2 rc1 rc2
+
 	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"
+	wait "$p1"; rc1=$?
+	wait "$p2"; rc2=$?
+	if [ $rc1 -eq 0 ] && [ $rc2 -eq 0 ]; then
+		tst_res TFAIL "measurement policy opened concurrently"
+	elif [ $rc1 -eq 0 ] || [ $rc2 -eq 0 ]; then
+		tst_res TPASS "replaced default measurement policy"
 	else
-		tst_resm TFAIL "problems opening measurement policy"
+		tst_res TFAIL "problems opening measurement policy"
 	fi
 }
 
-# Function:     test03
-# Description 	- Verify can't load another measurement policy.
-test03()
+test3()
 {
+	tst_res TINFO "verify that valid policy isn't replaced"
+
+	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..1a2df154d
--- a/testcases/kernel/security/integrity/ima/tests/ima_setup.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_setup.sh
@@ -1,86 +1,68 @@
 #!/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_CLEANUP="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
-}
+export TCID="${TCID:-$(basename $0 | cut -d. -f1)}"
 
-mount_securityfs()
-{
-	SECURITYFS=$(mount 2>/dev/null | awk '$5 == "securityfs" { print $3 }')
-	if [ "x$SECURITYFS" = x ] ; then
+SYSFS="/sys"
+UMOUNT=
 
-		SECURITYFS="$SYSFS/kernel/security"
+mount_helper()
+{
+	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()
 {
-	tst_require_root
-
-	tst_tmpdir
-
-	mount_sysfs
+	SECURITYFS="$(mount_helper securityfs $SYSFS/kernel/security)"
 
-	# 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
+	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"
 }
 
 cleanup()
 {
-	tst_rmdir
+	local dir
+	for dir in $UMOUNT; do
+		umount $dir
+	done
 }
+
+setup
diff --git a/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh b/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
index 333bf5f8a..e78926165 100755
--- a/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
@@ -1,70 +1,63 @@
 #!/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.
+
+TST_TESTFUNC="test"
+TST_SETUP="init"
+TST_CNT=3
+
+. ima_setup.sh
 
 init()
 {
 	tst_check_cmds ima_boot_aggregate ima_measure
 }
 
-# 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 +67,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)"
 
-	while read line ; do
+	local ima_measurements="$BINARY_MEASUREMENTS"
+	local aggregate_pcr="$(ima_measure $ima_measurements --validate)"
+	local dev_pcrs="$1"
+	local ret=0
+
+	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 TFAIL "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..879b6e949 100755
--- a/testcases/kernel/security/integrity/ima/tests/ima_violations.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_violations.sh
@@ -1,44 +1,49 @@
 #!/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_TESTFUNC="test"
+TST_SETUP="init"
+TST_CNT=3
 
-open_file_read()
+. ima_setup.sh
+
+. daemonlib.sh
+
+FILE="test.txt"
+IMA_VIOLATIONS="$SECURITYFS/ima/violations"
+
+init()
 {
-	exec 3< $1
-	if [ $? -ne 0 ]; then
-		exit 1
+	LOG="/var/log/messages"
+	SLEEP="1s"
+	if status_daemon auditd; then
+		LOG="/var/log/audit/audit.log"
+		SLEEP=
 	fi
+	tst_res TINFO "using log $LOG"
+}
+
+open_file_read()
+{
+	exec 3< $FILE || exit 1
 }
 
 close_file_read()
@@ -48,11 +53,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 +62,88 @@ 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
-
-	ima_violations=$SECURITYFS/ima/violations
+	local search="$1"
+	echo $(grep -c "${search}.*${FILE}" $LOG)
 }
 
-# Function:     test01
-# Description	- Verify open writers violation
-test01()
+validate()
 {
-	read num_violations < $ima_violations
-
-	TMPFN=test.txt
-	open_file_write $TMPFN
-	open_file_read $TMPFN
-	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)"
+	local num_violations="$1"
+	local count="$2"
+	local search="$3"
+	local count2="$(get_count $search)"
+	local num_violations_new
+
+	[ -n "$SLEEP" ] && tst_sleep $SLEEP
+
+	read num_violations_new < $IMA_VIOLATIONS
+	if [ $(($num_violations_new - $num_violations)) -gt 0 ]; then
+		if [ $count2 -gt $count ]; then
+			tst_res TPASS "$search violation added"
 		else
-			tst_resm TFAIL "(message ratelimiting?)"
+			tst_res TFAIL "$search not found in $LOG"
 		fi
 	else
-		tst_resm TFAIL "open_writers violation not added(test.txt)"
+		tst_res TFAIL "$search violation not added"
 	fi
 }
 
-# Function:     test02
-# Description   - Verify ToMToU violation
-test02()
+test1()
 {
-	read num_violations < $ima_violations
+	tst_res TINFO "verify open writers violation"
 
-	TMPFN=test.txt
-	open_file_read $TMPFN
-	open_file_write $TMPFN
-	close_file_write
+	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
-	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
+	close_file_write
+
+	validate $num_violations $count $search
 }
 
-# Function:     test03
-# Description 	- verify open_writers using mmapped files
-test03()
+test2()
 {
-	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
+	tst_res TINFO "verify ToMToU violation"
+
+	local search="ToMToU"
+	local count num_violations
+
+	read num_violations < $IMA_VIOLATIONS
+	count="$(get_count $search)"
+
+	open_file_read
+	open_file_write
+	close_file_write
 	close_file_read
+
+	validate $num_violations $count $search
 }
 
-. ima_setup.sh
+test3()
+{
+	tst_res TINFO "verify open_writers using mmapped files"
 
-setup
-TST_CLEANUP=cleanup
+	local search="open_writers"
+	local count num_violations
 
-init
-test01
-test02
-test03
+	read num_violations < $IMA_VIOLATIONS
+	count="$(get_count $search)"
+
+	echo 'testing testing ' > $FILE
+	ima_mmap $FILE &
+	tst_sleep 1s
+
+	open_file_read
+	close_file_read
+
+	validate $num_violations $count $search
+}
 
-tst_exit
+tst_run
-- 
2.16.2

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [LTP] [RFC PATCH v2 1/4] security/ima: Rewrite tests into new API + fixes
@ 2018-03-14 15:57   ` Petr Vorel
  0 siblings, 0 replies; 31+ messages in thread
From: Petr Vorel @ 2018-03-14 15:57 UTC (permalink / raw)
  To: ltp

* 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 occurance of tested file in
    /sys/kernel/security/ima/ascii_runtime_measurements
  - 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
  - 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) + add sleep when SUT uses
    /var/log/messages to prevent failure during validate

* 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

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
Changes v1->v2:
I increased sleep to 1s I still occasionally encounter errors on systems
logging into /var/log/messages:
ima_violations 2 TFAIL: ToMToU not found in /var/log/messages
---
 runtest/ima                                        |   8 +-
 .../integrity/ima/tests/ima_measurements.sh        | 239 +++++++++++----------
 .../security/integrity/ima/tests/ima_policy.sh     | 148 ++++++-------
 .../security/integrity/ima/tests/ima_setup.sh      | 110 ++++------
 .../kernel/security/integrity/ima/tests/ima_tpm.sh | 140 ++++++------
 .../security/integrity/ima/tests/ima_violations.sh | 217 +++++++++----------
 6 files changed, 409 insertions(+), 453 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..353e45b30 100755
--- a/testcases/kernel/security/integrity/ima/tests/ima_measurements.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_measurements.sh
@@ -1,139 +1,156 @@
 #!/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_TESTFUNC="test"
+TST_SETUP="init"
+TST_CNT=3
+TST_NEEDS_TMPDIR=1
+
+. ima_setup.sh
+
+DEFAULT_DIGEST_OLD_FORMAT="sha1"
+POLICY="$IMA_DIR/policy"
 
 init()
 {
-	tst_check_cmds sha1sum
+	tst_check_cmds awk
 
-	# verify using default policy
-	if [ ! -f "$IMA_DIR/policy" ]; then
-		tst_resm TINFO "not using default policy"
-	fi
+	[ -f "$POLICY" ] || tst_res TINFO "not using default policy"
+
+	TEST_FILE="$PWD/test.txt"
+	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
 
-	# Calculating the sha1sum of test.txt should add
-	# the new measurement to the measurement list
-	hash=$(sha1sum test.txt | sed 's/  -//')
+	line="$(grep $TEST_FILE $ASCII_MEASUREMENTS | tail -1)"
+	[ -n "$line" ] || tst_res TFAIL "cannot find measurement for '$TEST_FILE'"
 
-	# Check if the new measurement exists
-	cat /sys/kernel/security/ima/ascii_runtime_measurements > measurements
-	$(grep $hash measurements > /dev/null)
+	[ "$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)"
 
-	if [ $? -ne 0 ]; then
-		tst_resm TFAIL "Modified file not measured"
-		tst_resm TINFO "iversion not supported; or not mounted with iversion"
+	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 [ "$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()
+test1()
 {
-	# 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
+	tst_res TINFO "verify adding record to the IMA measurement list"
+	ROD echo "$(date) this is a test file" \> $TEST_FILE
+	ima_check
 }
 
-. ima_setup.sh
+test2()
+{
+	local device mount fs
+
+	tst_res TINFO "verify updating record in the IMA measurement list"
+
+	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
+		fi
+		;;
+	xfs)
+		if dmesg | grep -q "XFS.*Mounting V[1-4] Filesystem"; then
+			tst_res TCONF "XFS Filesystem >= V5 required for iversion support"
+			return
+		fi
+		;;
+	'')
+		tst_res TWARN "could not find mount info for device '$device'"
+		;;
+	esac
+
+	ROD echo "$(date) modified file" \> $TEST_FILE
+	ima_check
+}
+
+test3()
+{
+	local user="nobody"
+	local dir="$PWD/user"
+	local file="$dir/test.txt"
 
-setup
-TST_CLEANUP=cleanup
+	# Default policy does not measure user files
+	tst_res TINFO "verify not measuring user files"
 
-init
-test01
-test02
-test03
+	if ! id $user >/dev/null 2>/dev/null; then
+		tst_res TCONF "missing system user $user (wrong installation)"
+		return
+	fi
+	tst_check_cmds sudo
+
+	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..08e34c6f8 100755
--- a/testcases/kernel/security/integrity/ima/tests/ima_policy.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_policy.sh
@@ -1,127 +1,115 @@
 #!/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_TESTFUNC="test"
+TST_SETUP="init"
+TST_CNT=3
+
+. ima_setup.sh
 
 init()
 {
-	# 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 doesn't replace default policy"
+
+	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()
 {
+	tst_res TINFO "verify that policy file is opened sequentially and installs new policy"
+
+	local p1 p2 rc1 rc2
+
 	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"
+	wait "$p1"; rc1=$?
+	wait "$p2"; rc2=$?
+	if [ $rc1 -eq 0 ] && [ $rc2 -eq 0 ]; then
+		tst_res TFAIL "measurement policy opened concurrently"
+	elif [ $rc1 -eq 0 ] || [ $rc2 -eq 0 ]; then
+		tst_res TPASS "replaced default measurement policy"
 	else
-		tst_resm TFAIL "problems opening measurement policy"
+		tst_res TFAIL "problems opening measurement policy"
 	fi
 }
 
-# Function:     test03
-# Description 	- Verify can't load another measurement policy.
-test03()
+test3()
 {
+	tst_res TINFO "verify that valid policy isn't replaced"
+
+	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..1a2df154d
--- a/testcases/kernel/security/integrity/ima/tests/ima_setup.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_setup.sh
@@ -1,86 +1,68 @@
 #!/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_CLEANUP="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
-}
+export TCID="${TCID:-$(basename $0 | cut -d. -f1)}"
 
-mount_securityfs()
-{
-	SECURITYFS=$(mount 2>/dev/null | awk '$5 == "securityfs" { print $3 }')
-	if [ "x$SECURITYFS" = x ] ; then
+SYSFS="/sys"
+UMOUNT=
 
-		SECURITYFS="$SYSFS/kernel/security"
+mount_helper()
+{
+	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()
 {
-	tst_require_root
-
-	tst_tmpdir
-
-	mount_sysfs
+	SECURITYFS="$(mount_helper securityfs $SYSFS/kernel/security)"
 
-	# 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
+	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"
 }
 
 cleanup()
 {
-	tst_rmdir
+	local dir
+	for dir in $UMOUNT; do
+		umount $dir
+	done
 }
+
+setup
diff --git a/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh b/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
index 333bf5f8a..e78926165 100755
--- a/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
@@ -1,70 +1,63 @@
 #!/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.
+
+TST_TESTFUNC="test"
+TST_SETUP="init"
+TST_CNT=3
+
+. ima_setup.sh
 
 init()
 {
 	tst_check_cmds ima_boot_aggregate ima_measure
 }
 
-# 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 +67,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)"
 
-	while read line ; do
+	local ima_measurements="$BINARY_MEASUREMENTS"
+	local aggregate_pcr="$(ima_measure $ima_measurements --validate)"
+	local dev_pcrs="$1"
+	local ret=0
+
+	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 TFAIL "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..879b6e949 100755
--- a/testcases/kernel/security/integrity/ima/tests/ima_violations.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_violations.sh
@@ -1,44 +1,49 @@
 #!/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_TESTFUNC="test"
+TST_SETUP="init"
+TST_CNT=3
 
-open_file_read()
+. ima_setup.sh
+
+. daemonlib.sh
+
+FILE="test.txt"
+IMA_VIOLATIONS="$SECURITYFS/ima/violations"
+
+init()
 {
-	exec 3< $1
-	if [ $? -ne 0 ]; then
-		exit 1
+	LOG="/var/log/messages"
+	SLEEP="1s"
+	if status_daemon auditd; then
+		LOG="/var/log/audit/audit.log"
+		SLEEP=
 	fi
+	tst_res TINFO "using log $LOG"
+}
+
+open_file_read()
+{
+	exec 3< $FILE || exit 1
 }
 
 close_file_read()
@@ -48,11 +53,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 +62,88 @@ 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
-
-	ima_violations=$SECURITYFS/ima/violations
+	local search="$1"
+	echo $(grep -c "${search}.*${FILE}" $LOG)
 }
 
-# Function:     test01
-# Description	- Verify open writers violation
-test01()
+validate()
 {
-	read num_violations < $ima_violations
-
-	TMPFN=test.txt
-	open_file_write $TMPFN
-	open_file_read $TMPFN
-	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)"
+	local num_violations="$1"
+	local count="$2"
+	local search="$3"
+	local count2="$(get_count $search)"
+	local num_violations_new
+
+	[ -n "$SLEEP" ] && tst_sleep $SLEEP
+
+	read num_violations_new < $IMA_VIOLATIONS
+	if [ $(($num_violations_new - $num_violations)) -gt 0 ]; then
+		if [ $count2 -gt $count ]; then
+			tst_res TPASS "$search violation added"
 		else
-			tst_resm TFAIL "(message ratelimiting?)"
+			tst_res TFAIL "$search not found in $LOG"
 		fi
 	else
-		tst_resm TFAIL "open_writers violation not added(test.txt)"
+		tst_res TFAIL "$search violation not added"
 	fi
 }
 
-# Function:     test02
-# Description   - Verify ToMToU violation
-test02()
+test1()
 {
-	read num_violations < $ima_violations
+	tst_res TINFO "verify open writers violation"
 
-	TMPFN=test.txt
-	open_file_read $TMPFN
-	open_file_write $TMPFN
-	close_file_write
+	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
-	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
+	close_file_write
+
+	validate $num_violations $count $search
 }
 
-# Function:     test03
-# Description 	- verify open_writers using mmapped files
-test03()
+test2()
 {
-	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
+	tst_res TINFO "verify ToMToU violation"
+
+	local search="ToMToU"
+	local count num_violations
+
+	read num_violations < $IMA_VIOLATIONS
+	count="$(get_count $search)"
+
+	open_file_read
+	open_file_write
+	close_file_write
 	close_file_read
+
+	validate $num_violations $count $search
 }
 
-. ima_setup.sh
+test3()
+{
+	tst_res TINFO "verify open_writers using mmapped files"
 
-setup
-TST_CLEANUP=cleanup
+	local search="open_writers"
+	local count num_violations
 
-init
-test01
-test02
-test03
+	read num_violations < $IMA_VIOLATIONS
+	count="$(get_count $search)"
+
+	echo 'testing testing ' > $FILE
+	ima_mmap $FILE &
+	tst_sleep 1s
+
+	open_file_read
+	close_file_read
+
+	validate $num_violations $count $search
+}
 
-tst_exit
+tst_run
-- 
2.16.2


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [RFC PATCH v2 2/4] security/ima: Run measurements after policy
  2018-03-14 15:57 ` [LTP] " Petr Vorel
@ 2018-03-14 15:57   ` Petr Vorel
  -1 siblings, 0 replies; 31+ messages in thread
From: Petr Vorel @ 2018-03-14 15:57 UTC (permalink / raw)
  To: ltp; +Cc: Petr Vorel, Mimi Zohar, linux-integrity

This fixes failing policy tests when no IMA is configured on SUT.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
Mimi suggested in [1]:
The current ordering of the tests assume that the system was booted
with the builtin "ima_tcb" policy enabled on the boot command line.
Assuming that the kernel doesn't require policies to be signed,
changing the order of the tests is fine.  Or simply test whether the
system was booted with either "ima_tcb" or "ima_policy=tcb" boot
command line options.

Mimi, do I understand it correctly that ima_policy.sh should be called
first when using ima_tcb (original order) and second otherwise?
That would be problematic, as we need a fixed order of tests in runtest
file.

[1] http://lists.linux.it/pipermail/ltp/2018-January/007025.html
---
 runtest/ima | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [LTP] [RFC PATCH v2 2/4] security/ima: Run measurements after policy
@ 2018-03-14 15:57   ` Petr Vorel
  0 siblings, 0 replies; 31+ messages in thread
From: Petr Vorel @ 2018-03-14 15:57 UTC (permalink / raw)
  To: ltp

This fixes failing policy tests when no IMA is configured on SUT.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
Mimi suggested in [1]:
The current ordering of the tests assume that the system was booted
with the builtin "ima_tcb" policy enabled on the boot command line.
Assuming that the kernel doesn't require policies to be signed,
changing the order of the tests is fine.  Or simply test whether the
system was booted with either "ima_tcb" or "ima_policy=tcb" boot
command line options.

Mimi, do I understand it correctly that ima_policy.sh should be called
first when using ima_tcb (original order) and second otherwise?
That would be problematic, as we need a fixed order of tests in runtest
file.

[1] http://lists.linux.it/pipermail/ltp/2018-January/007025.html
---
 runtest/ima | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [RFC PATCH v2 3/4] ima/ima_boot_aggregate: Increase MAX_EVENT_SIZE to 8k
  2018-03-14 15:57 ` [LTP] " Petr Vorel
@ 2018-03-14 15:57   ` Petr Vorel
  -1 siblings, 0 replies; 31+ messages in thread
From: Petr Vorel @ 2018-03-14 15:57 UTC (permalink / raw)
  To: ltp; +Cc: Petr Vorel, Mimi Zohar, linux-integrity

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 hope 8k is enough.

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

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

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..c52cea4c9 100644
--- a/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
+++ b/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
@@ -30,7 +30,7 @@ char *TCID = "ima_boot_aggregate";
 #if HAVE_LIBCRYPTO
 #include <openssl/sha.h>
 
-#define MAX_EVENT_SIZE 500
+#define MAX_EVENT_SIZE 8192
 #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 */
-- 
2.16.2

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [LTP] [RFC PATCH v2 3/4] ima/ima_boot_aggregate: Increase MAX_EVENT_SIZE to 8k
@ 2018-03-14 15:57   ` Petr Vorel
  0 siblings, 0 replies; 31+ messages in thread
From: Petr Vorel @ 2018-03-14 15:57 UTC (permalink / raw)
  To: ltp

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 hope 8k is enough.

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

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

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..c52cea4c9 100644
--- a/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
+++ b/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
@@ -30,7 +30,7 @@ char *TCID = "ima_boot_aggregate";
 #if HAVE_LIBCRYPTO
 #include <openssl/sha.h>
 
-#define MAX_EVENT_SIZE 500
+#define MAX_EVENT_SIZE 8192
 #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 */
-- 
2.16.2


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [RFC PATCH v2 4/4] ima/tpm: Various fixes
  2018-03-14 15:57 ` [LTP] " Petr Vorel
@ 2018-03-14 15:57   ` Petr Vorel
  -1 siblings, 0 replies; 31+ messages in thread
From: Petr Vorel @ 2018-03-14 15:57 UTC (permalink / raw)
  To: ltp; +Cc: Petr Vorel, Mimi Zohar, linux-integrity

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

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 |  58 +++---
 3 files changed, 26 insertions(+), 252 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 e78926165..ced1383a1 100755
--- a/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
@@ -21,13 +21,13 @@
 
 TST_TESTFUNC="test"
 TST_SETUP="init"
-TST_CNT=3
+TST_CNT=2
 
 . ima_setup.sh
 
 init()
 {
-	tst_check_cmds ima_boot_aggregate ima_measure
+	tst_check_cmds awk cut evmctl ima_boot_aggregate
 }
 
 test1()
@@ -37,24 +37,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"
 
-		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"
@@ -69,29 +68,37 @@ 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}')"
+	[ -e "$aggregate_pcr" ] || tst_res TFAIL "failed to get PCR-10"
 
 	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_res TINFO "evmctl version: $(evmctl --version)"
 
-	# 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
+	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
+
+	if [ -f "$pcrs_path" ]; then
 		validate_pcr $pcrs_path
 		if [ $? -eq 0 ]; then
 			tst_res TPASS "aggregate PCR value matches real PCR value"
@@ -103,17 +110,4 @@ test2()
 	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"
-	fi
-}
-
 tst_run
-- 
2.16.2

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [LTP] [RFC PATCH v2 4/4] ima/tpm: Various fixes
@ 2018-03-14 15:57   ` Petr Vorel
  0 siblings, 0 replies; 31+ messages in thread
From: Petr Vorel @ 2018-03-14 15:57 UTC (permalink / raw)
  To: ltp

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

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 |  58 +++---
 3 files changed, 26 insertions(+), 252 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 e78926165..ced1383a1 100755
--- a/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
@@ -21,13 +21,13 @@
 
 TST_TESTFUNC="test"
 TST_SETUP="init"
-TST_CNT=3
+TST_CNT=2
 
 . ima_setup.sh
 
 init()
 {
-	tst_check_cmds ima_boot_aggregate ima_measure
+	tst_check_cmds awk cut evmctl ima_boot_aggregate
 }
 
 test1()
@@ -37,24 +37,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"
 
-		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"
@@ -69,29 +68,37 @@ 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}')"
+	[ -e "$aggregate_pcr" ] || tst_res TFAIL "failed to get PCR-10"
 
 	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_res TINFO "evmctl version: $(evmctl --version)"
 
-	# 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
+	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
+
+	if [ -f "$pcrs_path" ]; then
 		validate_pcr $pcrs_path
 		if [ $? -eq 0 ]; then
 			tst_res TPASS "aggregate PCR value matches real PCR value"
@@ -103,17 +110,4 @@ test2()
 	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"
-	fi
-}
-
 tst_run
-- 
2.16.2


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* Re: [RFC PATCH v2 1/4] security/ima: Rewrite tests into new API + fixes
  2018-03-14 15:57   ` [LTP] " Petr Vorel
@ 2018-03-14 16:32     ` Petr Vorel
  -1 siblings, 0 replies; 31+ messages in thread
From: Petr Vorel @ 2018-03-14 16:32 UTC (permalink / raw)
  To: ltp; +Cc: Mimi Zohar, linux-integrity

Hi,

> -# 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 TFAIL "TPM not enabled, no PCR value to validate"
Wrong, This must be TCONF:
		tst_res TCONF "TPM not enabled, no PCR value to validate"

>  	fi
>  }


Kind regards,
Petr

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [LTP] [RFC PATCH v2 1/4] security/ima: Rewrite tests into new API + fixes
@ 2018-03-14 16:32     ` Petr Vorel
  0 siblings, 0 replies; 31+ messages in thread
From: Petr Vorel @ 2018-03-14 16:32 UTC (permalink / raw)
  To: ltp

Hi,

> -# 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 TFAIL "TPM not enabled, no PCR value to validate"
Wrong, This must be TCONF:
		tst_res TCONF "TPM not enabled, no PCR value to validate"

>  	fi
>  }


Kind regards,
Petr

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [RFC PATCH v2 0/4] Rewrite tests into new API + fixes
  2018-03-14 15:57 ` [LTP] " Petr Vorel
@ 2018-03-26 22:31   ` Mimi Zohar
  -1 siblings, 0 replies; 31+ messages in thread
From: Mimi Zohar @ 2018-03-26 22:31 UTC (permalink / raw)
  To: Petr Vorel, ltp; +Cc: linux-integrity

Hi Petr,

On Wed, 2018-03-14 at 16:57 +0100, Petr Vorel wrote:
> Hi,
> 
> this is a second attempt to rewrite IMA tests.
> Comments and fixes are welcome.
> 
> Changes v1->v2:
> * ima_measurements.sh: add support for "ima-ng" and "ima-sig" IMA
>   measurement templates
> * ima_measurements.sh: add support for most of hash algorithms is
>   defined in include/uapi/linux/hash_info.h
> * fix ima_boot_aggregate ("ima/ima_boot_aggregate: Increase MAX_EVENT_SIZE to 8k")
> * ima_tpm.sh: fixes of TPM test ("ima/tpm: Various fixes")
> * ima_measurements.sh: drop ima_measure and use evmctl (external dependency) instead
> * ima_measurements.sh: check XFS version for iversion support
> 
> TODO
> * ima_measurements.sh: Add support for ima_template_fmt kernel parameter.
> 
> * ima_policy.sh: Detect if the policy must be signed [1] (IMA_WRITE_POLICY or
> "secure_boot" kernel parameter).

> @Mimi: What is a best approach in case policy must be signed? measure.policy
> and measure.policy-invalid files are not signed should we skip all tests in
> ima_policy.sh with something like "Not supported when policy must be signed"?
> Or running them both and expect them to fail as they're not signed?

> As that's how I understand your related commit in kernel
> (19f8a84713ed "ima: measure and appraise the IMA policy itself").

If you can determine that writing the policy should fail and then
detect that it actually did fail, the latter option is more complete. 

> BTW load_policy() use old approach catting the content into sysfs policy file.
> Maybe it'd be good to echo policy filename into sysfs policy file for kernel >
> 4.6 (feature added in 7429b092811f "ima: load policy using path").

Cat'ing the file doesn't work if the policy has to be signed. I
haven't tried writing the policy pathname, when the file doesn't
require it to be signed.  There's no reason that it wouldn't work.

> * ima_measurement.sh,ima_violations.sh: Avoid using tmpfs filesystem [1]. You
> suggested using RAM block device. Would it be ok to use filesystem created on
> loop device (/dev/loop0)? Or even create image file in $TMPDIR (mostly
> /tmp, which can be tmpfs) and use it as a loop device?

Yes, creating a filesystem on a loop device should work.

eg.
$ dd if=/dev/zero of=/tmp/test.img count=40960
$ mkfs -t ext3 -q /tmp/test.img
$ sudo mount -o loop=/dev/loop0 /tmp/test.img /mnt
$ sudo cp --preserve=xattr /usr/bin/ls /mnt/
$ getfattr -n security.ima -e hex --dump /mnt/ls

# file: mnt/ls
security.ima=0x0302046e6c104601001f0e4ed440b63a16bb13e68a3ae556a311047
57daa51bce56f505367c00ec05bbfe38a1f7b0aa56b86ff41eba7cddaca0b7522bb46e
00e72b63aeb81645c2e0fdf896dfc51303917bfb4c7b360ad22a85ff4a0570b2e29059
aba6c8ddd1db84d6cbfe6bb341549b7aa5a004acb96c4a14273ead954fb3ce3bcece04
680cba0c763b0d122d8f358a534cb2b786ef892d3729ff56abe10694c3bbcd35909cc1
046cb89553bb4636bc0493362758f24bb8cbaf5c88b16c0ecbb879ef7c4a3281cb970a
b34634923a5c73a624528f70a03ce5fbef2845e770c536a8da1cce374381a1b176a230
d438e9fd2f16f95c6ee16f49ba107cd57120a31f18339cfe3215a69


> To be honnest, I'm not sure if I addressed your comment [2]:
> These tests are for the IMA-measurement aspect only, not IMA-
> appraisal.  Adding measurements to the measurement list won't cause
> the system to stop working, unless keys are sealed to a particular TPM
> PCR value.  Nobody is or should be sealing keys to PCR-10, since the
> ordering of the measurements is non deterministic.
> As we add IMA-appraisal tests requiring files to be signed, things
> will fail if either the public key isn't on the IMA keyring or the
> file isn't properly signed.  For this reason, limiting file IMA-
> appraisal tests to a particular filesystem simplifies testing.

Even without adding IMA-appraisal tests, updating/cleaning up the IMA-
measurement tests is good.

thank you!

Mimi

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

> 
> Kind regards,
> Petr
> 
> 
> Petr Vorel (4):
>   security/ima: Rewrite tests into new API + fixes
>   security/ima: Run measurements after policy
>   ima/ima_boot_aggregate: Increase MAX_EVENT_SIZE to 8k
>   ima/tpm: Various fixes
> 
>  runtest/ima                                        |   8 +-
>  testcases/kernel/security/integrity/.gitignore     |   1 -
>  .../integrity/ima/src/ima_boot_aggregate.c         |   2 +-
>  .../security/integrity/ima/src/ima_measure.c       | 219 -------------------
>  .../integrity/ima/tests/ima_measurements.sh        | 239 +++++++++++----------
>  .../security/integrity/ima/tests/ima_policy.sh     | 148 ++++++-------
>  .../security/integrity/ima/tests/ima_setup.sh      | 110 ++++------
>  .../kernel/security/integrity/ima/tests/ima_tpm.sh | 160 ++++++--------
>  .../security/integrity/ima/tests/ima_violations.sh | 217 +++++++++----------
>  9 files changed, 417 insertions(+), 687 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
> 

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [LTP] [RFC PATCH v2 0/4] Rewrite tests into new API + fixes
@ 2018-03-26 22:31   ` Mimi Zohar
  0 siblings, 0 replies; 31+ messages in thread
From: Mimi Zohar @ 2018-03-26 22:31 UTC (permalink / raw)
  To: ltp

Hi Petr,

On Wed, 2018-03-14 at 16:57 +0100, Petr Vorel wrote:
> Hi,
> 
> this is a second attempt to rewrite IMA tests.
> Comments and fixes are welcome.
> 
> Changes v1->v2:
> * ima_measurements.sh: add support for "ima-ng" and "ima-sig" IMA
>   measurement templates
> * ima_measurements.sh: add support for most of hash algorithms is
>   defined in include/uapi/linux/hash_info.h
> * fix ima_boot_aggregate ("ima/ima_boot_aggregate: Increase MAX_EVENT_SIZE to 8k")
> * ima_tpm.sh: fixes of TPM test ("ima/tpm: Various fixes")
> * ima_measurements.sh: drop ima_measure and use evmctl (external dependency) instead
> * ima_measurements.sh: check XFS version for iversion support
> 
> TODO
> * ima_measurements.sh: Add support for ima_template_fmt kernel parameter.
> 
> * ima_policy.sh: Detect if the policy must be signed [1] (IMA_WRITE_POLICY or
> "secure_boot" kernel parameter).

> @Mimi: What is a best approach in case policy must be signed? measure.policy
> and measure.policy-invalid files are not signed should we skip all tests in
> ima_policy.sh with something like "Not supported when policy must be signed"?
> Or running them both and expect them to fail as they're not signed?

> As that's how I understand your related commit in kernel
> (19f8a84713ed "ima: measure and appraise the IMA policy itself").

If you can determine that writing the policy should fail and then
detect that it actually did fail, the latter option is more complete. 

> BTW load_policy() use old approach catting the content into sysfs policy file.
> Maybe it'd be good to echo policy filename into sysfs policy file for kernel >
> 4.6 (feature added in 7429b092811f "ima: load policy using path").

Cat'ing the file doesn't work if the policy has to be signed. I
haven't tried writing the policy pathname, when the file doesn't
require it to be signed.  There's no reason that it wouldn't work.

> * ima_measurement.sh,ima_violations.sh: Avoid using tmpfs filesystem [1]. You
> suggested using RAM block device. Would it be ok to use filesystem created on
> loop device (/dev/loop0)? Or even create image file in $TMPDIR (mostly
> /tmp, which can be tmpfs) and use it as a loop device?

Yes, creating a filesystem on a loop device should work.

eg.
$ dd if=/dev/zero of=/tmp/test.img count=40960
$ mkfs -t ext3 -q /tmp/test.img
$ sudo mount -o loop=/dev/loop0 /tmp/test.img /mnt
$ sudo cp --preserve=xattr /usr/bin/ls /mnt/
$ getfattr -n security.ima -e hex --dump /mnt/ls

# file: mnt/ls
security.ima=0x0302046e6c104601001f0e4ed440b63a16bb13e68a3ae556a311047
57daa51bce56f505367c00ec05bbfe38a1f7b0aa56b86ff41eba7cddaca0b7522bb46e
00e72b63aeb81645c2e0fdf896dfc51303917bfb4c7b360ad22a85ff4a0570b2e29059
aba6c8ddd1db84d6cbfe6bb341549b7aa5a004acb96c4a14273ead954fb3ce3bcece04
680cba0c763b0d122d8f358a534cb2b786ef892d3729ff56abe10694c3bbcd35909cc1
046cb89553bb4636bc0493362758f24bb8cbaf5c88b16c0ecbb879ef7c4a3281cb970a
b34634923a5c73a624528f70a03ce5fbef2845e770c536a8da1cce374381a1b176a230
d438e9fd2f16f95c6ee16f49ba107cd57120a31f18339cfe3215a69


> To be honnest, I'm not sure if I addressed your comment [2]:
> These tests are for the IMA-measurement aspect only, not IMA-
> appraisal.  Adding measurements to the measurement list won't cause
> the system to stop working, unless keys are sealed to a particular TPM
> PCR value.  Nobody is or should be sealing keys to PCR-10, since the
> ordering of the measurements is non deterministic.
> As we add IMA-appraisal tests requiring files to be signed, things
> will fail if either the public key isn't on the IMA keyring or the
> file isn't properly signed.  For this reason, limiting file IMA-
> appraisal tests to a particular filesystem simplifies testing.

Even without adding IMA-appraisal tests, updating/cleaning up the IMA-
measurement tests is good.

thank you!

Mimi

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

> 
> Kind regards,
> Petr
> 
> 
> Petr Vorel (4):
>   security/ima: Rewrite tests into new API + fixes
>   security/ima: Run measurements after policy
>   ima/ima_boot_aggregate: Increase MAX_EVENT_SIZE to 8k
>   ima/tpm: Various fixes
> 
>  runtest/ima                                        |   8 +-
>  testcases/kernel/security/integrity/.gitignore     |   1 -
>  .../integrity/ima/src/ima_boot_aggregate.c         |   2 +-
>  .../security/integrity/ima/src/ima_measure.c       | 219 -------------------
>  .../integrity/ima/tests/ima_measurements.sh        | 239 +++++++++++----------
>  .../security/integrity/ima/tests/ima_policy.sh     | 148 ++++++-------
>  .../security/integrity/ima/tests/ima_setup.sh      | 110 ++++------
>  .../kernel/security/integrity/ima/tests/ima_tpm.sh | 160 ++++++--------
>  .../security/integrity/ima/tests/ima_violations.sh | 217 +++++++++----------
>  9 files changed, 417 insertions(+), 687 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
> 


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [RFC PATCH v2 0/4] Rewrite tests into new API + fixes
  2018-03-26 22:31   ` [LTP] " Mimi Zohar
@ 2018-03-27  9:22     ` Petr Vorel
  -1 siblings, 0 replies; 31+ messages in thread
From: Petr Vorel @ 2018-03-27  9:22 UTC (permalink / raw)
  To: Mimi Zohar; +Cc: ltp, linux-integrity

Hi Mimi,

> Hi Petr,

> On Wed, 2018-03-14 at 16:57 +0100, Petr Vorel wrote:
> > Hi,

> > this is a second attempt to rewrite IMA tests.
> > Comments and fixes are welcome.

> > Changes v1->v2:
> > * ima_measurements.sh: add support for "ima-ng" and "ima-sig" IMA
> >   measurement templates
> > * ima_measurements.sh: add support for most of hash algorithms is
> >   defined in include/uapi/linux/hash_info.h
> > * fix ima_boot_aggregate ("ima/ima_boot_aggregate: Increase MAX_EVENT_SIZE to 8k")
> > * ima_tpm.sh: fixes of TPM test ("ima/tpm: Various fixes")
> > * ima_measurements.sh: drop ima_measure and use evmctl (external dependency) instead
> > * ima_measurements.sh: check XFS version for iversion support

> > TODO
> > * ima_measurements.sh: Add support for ima_template_fmt kernel parameter.

> > * ima_policy.sh: Detect if the policy must be signed [1] (IMA_WRITE_POLICY or
> > "secure_boot" kernel parameter).

> > @Mimi: What is a best approach in case policy must be signed? measure.policy
> > and measure.policy-invalid files are not signed should we skip all tests in
> > ima_policy.sh with something like "Not supported when policy must be signed"?
> > Or running them both and expect them to fail as they're not signed?

> > As that's how I understand your related commit in kernel
> > (19f8a84713ed "ima: measure and appraise the IMA policy itself").

> If you can determine that writing the policy should fail and then
> detect that it actually did fail, the latter option is more complete. 
I'll try.

> > BTW load_policy() use old approach catting the content into sysfs policy file.
> > Maybe it'd be good to echo policy filename into sysfs policy file for kernel >
> > 4.6 (feature added in 7429b092811f "ima: load policy using path").

> Cat'ing the file doesn't work if the policy has to be signed. I
> haven't tried writing the policy pathname, when the file doesn't
> require it to be signed.  There's no reason that it wouldn't work.
Ideal testsuite would tests all combinations, but thats impossible as that's too complex
(setting kernel variables in grub and rebooting is clearly out of LTP scope).
I might try to cover more scenarios (e.g. try to cat policy when required to be signed and
detect failure before uploading policy pathname; trying to append policy for second time
when it's not allowed).

> > * ima_measurement.sh,ima_violations.sh: Avoid using tmpfs filesystem [1]. You
> > suggested using RAM block device. Would it be ok to use filesystem created on
> > loop device (/dev/loop0)? Or even create image file in $TMPDIR (mostly
> > /tmp, which can be tmpfs) and use it as a loop device?

> Yes, creating a filesystem on a loop device should work.

> eg.
> $ dd if=/dev/zero of=/tmp/test.img count=40960
> $ mkfs -t ext3 -q /tmp/test.img
> $ sudo mount -o loop=/dev/loop0 /tmp/test.img /mnt
> $ sudo cp --preserve=xattr /usr/bin/ls /mnt/
> $ getfattr -n security.ima -e hex --dump /mnt/ls

> # file: mnt/ls
> security.ima=0x0302046e6c104601001f0e4ed440b63a16bb13e68a3ae556a311047
> 57daa51bce56f505367c00ec05bbfe38a1f7b0aa56b86ff41eba7cddaca0b7522bb46e
> 00e72b63aeb81645c2e0fdf896dfc51303917bfb4c7b360ad22a85ff4a0570b2e29059
> aba6c8ddd1db84d6cbfe6bb341549b7aa5a004acb96c4a14273ead954fb3ce3bcece04
> 680cba0c763b0d122d8f358a534cb2b786ef892d3729ff56abe10694c3bbcd35909cc1
> 046cb89553bb4636bc0493362758f24bb8cbaf5c88b16c0ecbb879ef7c4a3281cb970a
> b34634923a5c73a624528f70a03ce5fbef2845e770c536a8da1cce374381a1b176a230
> d438e9fd2f16f95c6ee16f49ba107cd57120a31f18339cfe3215a69

Thanks for a complete howto :).


> > To be honnest, I'm not sure if I addressed your comment [2]:
> > These tests are for the IMA-measurement aspect only, not IMA-
> > appraisal.  Adding measurements to the measurement list won't cause
> > the system to stop working, unless keys are sealed to a particular TPM
> > PCR value.  Nobody is or should be sealing keys to PCR-10, since the
> > ordering of the measurements is non deterministic.
> > As we add IMA-appraisal tests requiring files to be signed, things
> > will fail if either the public key isn't on the IMA keyring or the
> > file isn't properly signed.  For this reason, limiting file IMA-
> > appraisal tests to a particular filesystem simplifies testing.

> Even without adding IMA-appraisal tests, updating/cleaning up the IMA-
> measurement tests is good.
I'll merge current version (note issues in TODO in commit message) and work these issues
when time allows.
Can I add you Acked-by?

> thank you!
Thanks a lot for your comments and time.

> Mimi


Kind regards,
Petr

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



> > Kind regards,
> > Petr


> > Petr Vorel (4):
> >   security/ima: Rewrite tests into new API + fixes
> >   security/ima: Run measurements after policy
> >   ima/ima_boot_aggregate: Increase MAX_EVENT_SIZE to 8k
> >   ima/tpm: Various fixes

> >  runtest/ima                                        |   8 +-
> >  testcases/kernel/security/integrity/.gitignore     |   1 -
> >  .../integrity/ima/src/ima_boot_aggregate.c         |   2 +-
> >  .../security/integrity/ima/src/ima_measure.c       | 219 -------------------
> >  .../integrity/ima/tests/ima_measurements.sh        | 239 +++++++++++----------
> >  .../security/integrity/ima/tests/ima_policy.sh     | 148 ++++++-------
> >  .../security/integrity/ima/tests/ima_setup.sh      | 110 ++++------
> >  .../kernel/security/integrity/ima/tests/ima_tpm.sh | 160 ++++++--------
> >  .../security/integrity/ima/tests/ima_violations.sh | 217 +++++++++----------
> >  9 files changed, 417 insertions(+), 687 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

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [LTP] [RFC PATCH v2 0/4] Rewrite tests into new API + fixes
@ 2018-03-27  9:22     ` Petr Vorel
  0 siblings, 0 replies; 31+ messages in thread
From: Petr Vorel @ 2018-03-27  9:22 UTC (permalink / raw)
  To: ltp

Hi Mimi,

> Hi Petr,

> On Wed, 2018-03-14 at 16:57 +0100, Petr Vorel wrote:
> > Hi,

> > this is a second attempt to rewrite IMA tests.
> > Comments and fixes are welcome.

> > Changes v1->v2:
> > * ima_measurements.sh: add support for "ima-ng" and "ima-sig" IMA
> >   measurement templates
> > * ima_measurements.sh: add support for most of hash algorithms is
> >   defined in include/uapi/linux/hash_info.h
> > * fix ima_boot_aggregate ("ima/ima_boot_aggregate: Increase MAX_EVENT_SIZE to 8k")
> > * ima_tpm.sh: fixes of TPM test ("ima/tpm: Various fixes")
> > * ima_measurements.sh: drop ima_measure and use evmctl (external dependency) instead
> > * ima_measurements.sh: check XFS version for iversion support

> > TODO
> > * ima_measurements.sh: Add support for ima_template_fmt kernel parameter.

> > * ima_policy.sh: Detect if the policy must be signed [1] (IMA_WRITE_POLICY or
> > "secure_boot" kernel parameter).

> > @Mimi: What is a best approach in case policy must be signed? measure.policy
> > and measure.policy-invalid files are not signed should we skip all tests in
> > ima_policy.sh with something like "Not supported when policy must be signed"?
> > Or running them both and expect them to fail as they're not signed?

> > As that's how I understand your related commit in kernel
> > (19f8a84713ed "ima: measure and appraise the IMA policy itself").

> If you can determine that writing the policy should fail and then
> detect that it actually did fail, the latter option is more complete. 
I'll try.

> > BTW load_policy() use old approach catting the content into sysfs policy file.
> > Maybe it'd be good to echo policy filename into sysfs policy file for kernel >
> > 4.6 (feature added in 7429b092811f "ima: load policy using path").

> Cat'ing the file doesn't work if the policy has to be signed. I
> haven't tried writing the policy pathname, when the file doesn't
> require it to be signed.  There's no reason that it wouldn't work.
Ideal testsuite would tests all combinations, but thats impossible as that's too complex
(setting kernel variables in grub and rebooting is clearly out of LTP scope).
I might try to cover more scenarios (e.g. try to cat policy when required to be signed and
detect failure before uploading policy pathname; trying to append policy for second time
when it's not allowed).

> > * ima_measurement.sh,ima_violations.sh: Avoid using tmpfs filesystem [1]. You
> > suggested using RAM block device. Would it be ok to use filesystem created on
> > loop device (/dev/loop0)? Or even create image file in $TMPDIR (mostly
> > /tmp, which can be tmpfs) and use it as a loop device?

> Yes, creating a filesystem on a loop device should work.

> eg.
> $ dd if=/dev/zero of=/tmp/test.img count=40960
> $ mkfs -t ext3 -q /tmp/test.img
> $ sudo mount -o loop=/dev/loop0 /tmp/test.img /mnt
> $ sudo cp --preserve=xattr /usr/bin/ls /mnt/
> $ getfattr -n security.ima -e hex --dump /mnt/ls

> # file: mnt/ls
> security.ima=0x0302046e6c104601001f0e4ed440b63a16bb13e68a3ae556a311047
> 57daa51bce56f505367c00ec05bbfe38a1f7b0aa56b86ff41eba7cddaca0b7522bb46e
> 00e72b63aeb81645c2e0fdf896dfc51303917bfb4c7b360ad22a85ff4a0570b2e29059
> aba6c8ddd1db84d6cbfe6bb341549b7aa5a004acb96c4a14273ead954fb3ce3bcece04
> 680cba0c763b0d122d8f358a534cb2b786ef892d3729ff56abe10694c3bbcd35909cc1
> 046cb89553bb4636bc0493362758f24bb8cbaf5c88b16c0ecbb879ef7c4a3281cb970a
> b34634923a5c73a624528f70a03ce5fbef2845e770c536a8da1cce374381a1b176a230
> d438e9fd2f16f95c6ee16f49ba107cd57120a31f18339cfe3215a69

Thanks for a complete howto :).


> > To be honnest, I'm not sure if I addressed your comment [2]:
> > These tests are for the IMA-measurement aspect only, not IMA-
> > appraisal.  Adding measurements to the measurement list won't cause
> > the system to stop working, unless keys are sealed to a particular TPM
> > PCR value.  Nobody is or should be sealing keys to PCR-10, since the
> > ordering of the measurements is non deterministic.
> > As we add IMA-appraisal tests requiring files to be signed, things
> > will fail if either the public key isn't on the IMA keyring or the
> > file isn't properly signed.  For this reason, limiting file IMA-
> > appraisal tests to a particular filesystem simplifies testing.

> Even without adding IMA-appraisal tests, updating/cleaning up the IMA-
> measurement tests is good.
I'll merge current version (note issues in TODO in commit message) and work these issues
when time allows.
Can I add you Acked-by?

> thank you!
Thanks a lot for your comments and time.

> Mimi


Kind regards,
Petr

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



> > Kind regards,
> > Petr


> > Petr Vorel (4):
> >   security/ima: Rewrite tests into new API + fixes
> >   security/ima: Run measurements after policy
> >   ima/ima_boot_aggregate: Increase MAX_EVENT_SIZE to 8k
> >   ima/tpm: Various fixes

> >  runtest/ima                                        |   8 +-
> >  testcases/kernel/security/integrity/.gitignore     |   1 -
> >  .../integrity/ima/src/ima_boot_aggregate.c         |   2 +-
> >  .../security/integrity/ima/src/ima_measure.c       | 219 -------------------
> >  .../integrity/ima/tests/ima_measurements.sh        | 239 +++++++++++----------
> >  .../security/integrity/ima/tests/ima_policy.sh     | 148 ++++++-------
> >  .../security/integrity/ima/tests/ima_setup.sh      | 110 ++++------
> >  .../kernel/security/integrity/ima/tests/ima_tpm.sh | 160 ++++++--------
> >  .../security/integrity/ima/tests/ima_violations.sh | 217 +++++++++----------
> >  9 files changed, 417 insertions(+), 687 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



^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [RFC PATCH v2 1/4] security/ima: Rewrite tests into new API + fixes
  2018-03-14 15:57   ` [LTP] " Petr Vorel
@ 2018-03-27 19:12     ` Mimi Zohar
  -1 siblings, 0 replies; 31+ messages in thread
From: Mimi Zohar @ 2018-03-27 19:12 UTC (permalink / raw)
  To: Petr Vorel, ltp; +Cc: linux-integrity

On Wed, 2018-03-14 at 16:57 +0100, Petr Vorel wrote:
> * 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 occurance of tested file in
>     /sys/kernel/security/ima/ascii_runtime_measurements
>   - 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

Needing the filesystem to be mounted with i_version is changing in
Linux 4.16.  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 (based on policy), making
i_version a performance improvement.

[...]

>  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
> -	}

Originally writing the policy was done one rule at a time, but hasn't
been required for a long time.  dracut and systemd 'cat' the policy
directly to the pseudo file.

Mimi

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [LTP] [RFC PATCH v2 1/4] security/ima: Rewrite tests into new API + fixes
@ 2018-03-27 19:12     ` Mimi Zohar
  0 siblings, 0 replies; 31+ messages in thread
From: Mimi Zohar @ 2018-03-27 19:12 UTC (permalink / raw)
  To: ltp

On Wed, 2018-03-14 at 16:57 +0100, Petr Vorel wrote:
> * 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 occurance of tested file in
>     /sys/kernel/security/ima/ascii_runtime_measurements
>   - 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

Needing the filesystem to be mounted with i_version is changing in
Linux 4.16.  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 (based on policy), making
i_version a performance improvement.

[...]

>  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
> -	}

Originally writing the policy was done one rule at a time, but hasn't
been required for a long time.  dracut and systemd 'cat' the policy
directly to the pseudo file.

Mimi


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [RFC PATCH v2 3/4] ima/ima_boot_aggregate: Increase MAX_EVENT_SIZE to 8k
  2018-03-14 15:57   ` [LTP] " Petr Vorel
@ 2018-03-27 19:44     ` Mimi Zohar
  -1 siblings, 0 replies; 31+ messages in thread
From: Mimi Zohar @ 2018-03-27 19:44 UTC (permalink / raw)
  To: Petr Vorel, ltp; +Cc: linux-integrity, George Wilson

[Cc'ing George Wilson]

On Wed, 2018-03-14 at 16:57 +0100, Petr Vorel wrote:
> 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 hope 8k is enough.

Is there a way of making this value system dependent?  On my 
laptop this is fine, but for PowerVM w/TPM 1.2 I've been told this is
too small.

> [1] http://lists.linux.it/pipermail/ltp/2018-January/006970.html
> [2] http://lists.linux.it/pipermail/ltp/2018-January/007002.html
> 
> Signed-off-by: Petr Vorel <pvorel@suse.cz>

Acked-by: Mimi Zohar <zohar@linux.vnet.ibm.com>

> ---
>  testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> 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..c52cea4c9 100644
> --- a/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
> +++ b/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
> @@ -30,7 +30,7 @@ char *TCID = "ima_boot_aggregate";
>  #if HAVE_LIBCRYPTO
>  #include <openssl/sha.h>
> 
> -#define MAX_EVENT_SIZE 500
> +#define MAX_EVENT_SIZE 8192
>  #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 */

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [LTP] [RFC PATCH v2 3/4] ima/ima_boot_aggregate: Increase MAX_EVENT_SIZE to 8k
@ 2018-03-27 19:44     ` Mimi Zohar
  0 siblings, 0 replies; 31+ messages in thread
From: Mimi Zohar @ 2018-03-27 19:44 UTC (permalink / raw)
  To: ltp

[Cc'ing George Wilson]

On Wed, 2018-03-14 at 16:57 +0100, Petr Vorel wrote:
> 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 hope 8k is enough.

Is there a way of making this value system dependent?  On my 
laptop this is fine, but for PowerVM w/TPM 1.2 I've been told this is
too small.

> [1] http://lists.linux.it/pipermail/ltp/2018-January/006970.html
> [2] http://lists.linux.it/pipermail/ltp/2018-January/007002.html
> 
> Signed-off-by: Petr Vorel <pvorel@suse.cz>

Acked-by: Mimi Zohar <zohar@linux.vnet.ibm.com>

> ---
>  testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> 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..c52cea4c9 100644
> --- a/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
> +++ b/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
> @@ -30,7 +30,7 @@ char *TCID = "ima_boot_aggregate";
>  #if HAVE_LIBCRYPTO
>  #include <openssl/sha.h>
> 
> -#define MAX_EVENT_SIZE 500
> +#define MAX_EVENT_SIZE 8192
>  #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 */


^ permalink raw reply	[flat|nested] 31+ messages in thread

* [LTP] [RFC PATCH v2 3/4] ima/ima_boot_aggregate: Increase MAX_EVENT_SIZE to 8k
  2018-03-27 19:44     ` [LTP] " Mimi Zohar
  (?)
@ 2018-03-27 22:23     ` George Wilson
  2018-03-29  6:18         ` [LTP] " Petr Vorel
  -1 siblings, 1 reply; 31+ messages in thread
From: George Wilson @ 2018-03-27 22:23 UTC (permalink / raw)
  To: ltp


Mimi Zohar <zohar@linux.vnet.ibm.com> wrote on 03/27/2018 02:44:15 PM:

> From: Mimi Zohar <zohar@linux.vnet.ibm.com>
> To: Petr Vorel <pvorel@suse.cz>, ltp@lists.linux.it
> Cc: linux-integrity@vger.kernel.org, George Wilson/Austin/IBM@IBMUS
> Date: 03/27/2018 02:44 PM
> Subject: Re: [RFC PATCH v2 3/4] ima/ima_boot_aggregate: Increase
MAX_EVENT_SIZE to 8k
>
> [Cc'ing George Wilson]
>
> On Wed, 2018-03-14 at 16:57 +0100, Petr Vorel wrote:
> > 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 hope 8k is enough.
>
> Is there a way of making this value system dependent?  On my
> laptop this is fine, but for PowerVM w/TPM 1.2 I've been told this is
> too small.

Why not follow the spec?  PowerVM has enormous events because they
were allowed by the 1.2 spec.  The 2.0 spec recommends 1M so I think
they should be at least 1M.  Because they're large, they should really
be dynamically allocated.

>
> > [1] http://lists.linux.it/pipermail/ltp/2018-January/006970.html
> > [2] http://lists.linux.it/pipermail/ltp/2018-January/007002.html
> >
> > Signed-off-by: Petr Vorel <pvorel@suse.cz>
>
> Acked-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
>
> > ---
> >  testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c | 2
+-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > 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..c52cea4c9 100644
> > --- a/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
> > +++ b/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
> > @@ -30,7 +30,7 @@ char *TCID = "ima_boot_aggregate";
> >  #if HAVE_LIBCRYPTO
> >  #include <openssl/sha.h>
> >
> > -#define MAX_EVENT_SIZE 500
> > +#define MAX_EVENT_SIZE 8192
> >  #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 */
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20180327/2a7825a3/attachment-0001.html>

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [RFC PATCH v2 3/4] ima/ima_boot_aggregate: Increase MAX_EVENT_SIZE to 8k
  2018-03-27 22:23     ` George Wilson
@ 2018-03-29  6:18         ` Petr Vorel
  0 siblings, 0 replies; 31+ messages in thread
From: Petr Vorel @ 2018-03-29  6:18 UTC (permalink / raw)
  To: George Wilson; +Cc: Mimi Zohar, linux-integrity, ltp

Hi George,

> > Is there a way of making this value system dependent?  On my
> > laptop this is fine, but for PowerVM w/TPM 1.2 I've been told this is
> > too small.

> Why not follow the spec?  PowerVM has enormous events because they
> were allowed by the 1.2 spec.  The 2.0 spec recommends 1M so I think
> they should be at least 1M.  Because they're large, they should really
> be dynamically allocated.
Make sense. Lets try 1M.
Thanks a lot for your input.


Kind regards,
Petr

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [LTP] [RFC PATCH v2 3/4] ima/ima_boot_aggregate: Increase MAX_EVENT_SIZE to 8k
@ 2018-03-29  6:18         ` Petr Vorel
  0 siblings, 0 replies; 31+ messages in thread
From: Petr Vorel @ 2018-03-29  6:18 UTC (permalink / raw)
  To: ltp

Hi George,

> > Is there a way of making this value system dependent?  On my
> > laptop this is fine, but for PowerVM w/TPM 1.2 I've been told this is
> > too small.

> Why not follow the spec?  PowerVM has enormous events because they
> were allowed by the 1.2 spec.  The 2.0 spec recommends 1M so I think
> they should be at least 1M.  Because they're large, they should really
> be dynamically allocated.
Make sense. Lets try 1M.
Thanks a lot for your input.


Kind regards,
Petr

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [RFC PATCH v2 1/4] security/ima: Rewrite tests into new API + fixes
  2018-03-27 19:12     ` [LTP] " Mimi Zohar
@ 2018-03-29  8:59       ` Petr Vorel
  -1 siblings, 0 replies; 31+ messages in thread
From: Petr Vorel @ 2018-03-29  8:59 UTC (permalink / raw)
  To: Mimi Zohar; +Cc: ltp, linux-integrity

Hi Mimi,

> > * 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 occurance of tested file in
> >     /sys/kernel/security/ima/ascii_runtime_measurements
> >   - 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

> Needing the filesystem to be mounted with i_version is changing in
> Linux 4.16.  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 (based on policy), making
> i_version a performance improvement.
Thanks for info, I'll update the test.

> >  load_policy()
...
> >  	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
> > -	}

> Originally writing the policy was done one rule at a time, but hasn't
> been required for a long time.  dracut and systemd 'cat' the policy
> directly to the pseudo file.
OK, let's simplify it to catting the content.

> Mimi


Kind regards,
Petr

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [LTP] [RFC PATCH v2 1/4] security/ima: Rewrite tests into new API + fixes
@ 2018-03-29  8:59       ` Petr Vorel
  0 siblings, 0 replies; 31+ messages in thread
From: Petr Vorel @ 2018-03-29  8:59 UTC (permalink / raw)
  To: ltp

Hi Mimi,

> > * 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 occurance of tested file in
> >     /sys/kernel/security/ima/ascii_runtime_measurements
> >   - 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

> Needing the filesystem to be mounted with i_version is changing in
> Linux 4.16.  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 (based on policy), making
> i_version a performance improvement.
Thanks for info, I'll update the test.

> >  load_policy()
...
> >  	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
> > -	}

> Originally writing the policy was done one rule at a time, but hasn't
> been required for a long time.  dracut and systemd 'cat' the policy
> directly to the pseudo file.
OK, let's simplify it to catting the content.

> Mimi


Kind regards,
Petr


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [RFC PATCH v2 1/4] security/ima: Rewrite tests into new API + fixes
  2018-03-29  8:59       ` [LTP] " Petr Vorel
@ 2018-04-10 15:56         ` Mimi Zohar
  -1 siblings, 0 replies; 31+ messages in thread
From: Mimi Zohar @ 2018-04-10 15:56 UTC (permalink / raw)
  To: Petr Vorel; +Cc: ltp, linux-integrity

On Thu, 2018-03-29 at 10:59 +0200, Petr Vorel wrote:
> Hi Mimi,

> > >  load_policy()
> ...
> > >  	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
> > > -	}
> 
> > Originally writing the policy was done one rule at a time, but hasn't
> > been required for a long time.  dracut and systemd 'cat' the policy
> > directly to the pseudo file.
> OK, let's simplify it to catting the content.

Replacing the builtin policy with a new policy in the initramfs was
considered safe.  With commit 38d859f991f3 ("IMA: policy can now be
updated multiple times") the policy can be extended multiple times,
not only from the initramfs.  For it to be safe to extend the IMA
policy (eg. CONFIG_IMA_WRITE_POLICY), the policy must be signed.

These tests assume the policy does not need to be signed.

Mimi

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [LTP] [RFC PATCH v2 1/4] security/ima: Rewrite tests into new API + fixes
@ 2018-04-10 15:56         ` Mimi Zohar
  0 siblings, 0 replies; 31+ messages in thread
From: Mimi Zohar @ 2018-04-10 15:56 UTC (permalink / raw)
  To: ltp

On Thu, 2018-03-29 at 10:59 +0200, Petr Vorel wrote:
> Hi Mimi,

> > >  load_policy()
> ...
> > >  	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
> > > -	}
> 
> > Originally writing the policy was done one rule at a time, but hasn't
> > been required for a long time.  dracut and systemd 'cat' the policy
> > directly to the pseudo file.
> OK, let's simplify it to catting the content.

Replacing the builtin policy with a new policy in the initramfs was
considered safe.  With commit 38d859f991f3 ("IMA: policy can now be
updated multiple times") the policy can be extended multiple times,
not only from the initramfs.  For it to be safe to extend the IMA
policy (eg. CONFIG_IMA_WRITE_POLICY), the policy must be signed.

These tests assume the policy does not need to be signed.

Mimi


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [RFC PATCH v2 1/4] security/ima: Rewrite tests into new API + fixes
  2018-04-10 15:56         ` [LTP] " Mimi Zohar
@ 2018-04-11 19:03           ` Petr Vorel
  -1 siblings, 0 replies; 31+ messages in thread
From: Petr Vorel @ 2018-04-11 19:03 UTC (permalink / raw)
  To: Mimi Zohar; +Cc: ltp, linux-integrity

Hi Mimi,

> > > >  load_policy()
> > ...
> > > >  	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
> > > > -	}

> > > Originally writing the policy was done one rule at a time, but hasn't
> > > been required for a long time.  dracut and systemd 'cat' the policy
> > > directly to the pseudo file.
> > OK, let's simplify it to catting the content.

> Replacing the builtin policy with a new policy in the initramfs was
> considered safe.  With commit 38d859f991f3 ("IMA: policy can now be
> updated multiple times") the policy can be extended multiple times,
> not only from the initramfs.  For it to be safe to extend the IMA
> policy (eg. CONFIG_IMA_WRITE_POLICY), the policy must be signed.

> These tests assume the policy does not need to be signed.

Is it a good idea to expect that policy must be signed also for older kernels
(kernels before 4.5)?

> Mimi


Kind regards,
Petr

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [LTP] [RFC PATCH v2 1/4] security/ima: Rewrite tests into new API + fixes
@ 2018-04-11 19:03           ` Petr Vorel
  0 siblings, 0 replies; 31+ messages in thread
From: Petr Vorel @ 2018-04-11 19:03 UTC (permalink / raw)
  To: ltp

Hi Mimi,

> > > >  load_policy()
> > ...
> > > >  	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
> > > > -	}

> > > Originally writing the policy was done one rule at a time, but hasn't
> > > been required for a long time.  dracut and systemd 'cat' the policy
> > > directly to the pseudo file.
> > OK, let's simplify it to catting the content.

> Replacing the builtin policy with a new policy in the initramfs was
> considered safe.  With commit 38d859f991f3 ("IMA: policy can now be
> updated multiple times") the policy can be extended multiple times,
> not only from the initramfs.  For it to be safe to extend the IMA
> policy (eg. CONFIG_IMA_WRITE_POLICY), the policy must be signed.

> These tests assume the policy does not need to be signed.

Is it a good idea to expect that policy must be signed also for older kernels
(kernels before 4.5)?

> Mimi


Kind regards,
Petr

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [RFC PATCH v2 1/4] security/ima: Rewrite tests into new API + fixes
  2018-04-11 19:03           ` [LTP] " Petr Vorel
@ 2018-04-11 20:03             ` Mimi Zohar
  -1 siblings, 0 replies; 31+ messages in thread
From: Mimi Zohar @ 2018-04-11 20:03 UTC (permalink / raw)
  To: Petr Vorel; +Cc: ltp, linux-integrity

On Wed, 2018-04-11 at 21:03 +0200, Petr Vorel wrote:
> Hi Mimi,
> > > > >  load_policy()
> > > ...
> > > > >  	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
> > > > > -	}
> 
> > > > Originally writing the policy was done one rule at a time, but hasn't
> > > > been required for a long time.  dracut and systemd 'cat' the policy
> > > > directly to the pseudo file.
> > > OK, let's simplify it to catting the content.
> 
> > Replacing the builtin policy with a new policy in the initramfs was
> > considered safe.  With commit 38d859f991f3 ("IMA: policy can now be
> > updated multiple times") the policy can be extended multiple times,
> > not only from the initramfs.  For it to be safe to extend the IMA
> > policy (eg. CONFIG_IMA_WRITE_POLICY), the policy must be signed.
> 
> > These tests assume the policy does not need to be signed.

> Is it a good idea to expect that policy must be signed also for older kernels
> (kernels before 4.5)?

The ability to sign the policy file was introduced with commit 7429b09
("ima: load policy using path").  According to "git branch --
contains", it was upstreamed in linux-4.6.

Mimi

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [LTP] [RFC PATCH v2 1/4] security/ima: Rewrite tests into new API + fixes
@ 2018-04-11 20:03             ` Mimi Zohar
  0 siblings, 0 replies; 31+ messages in thread
From: Mimi Zohar @ 2018-04-11 20:03 UTC (permalink / raw)
  To: ltp

On Wed, 2018-04-11 at 21:03 +0200, Petr Vorel wrote:
> Hi Mimi,
> > > > >  load_policy()
> > > ...
> > > > >  	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
> > > > > -	}
> 
> > > > Originally writing the policy was done one rule at a time, but hasn't
> > > > been required for a long time.  dracut and systemd 'cat' the policy
> > > > directly to the pseudo file.
> > > OK, let's simplify it to catting the content.
> 
> > Replacing the builtin policy with a new policy in the initramfs was
> > considered safe.  With commit 38d859f991f3 ("IMA: policy can now be
> > updated multiple times") the policy can be extended multiple times,
> > not only from the initramfs.  For it to be safe to extend the IMA
> > policy (eg. CONFIG_IMA_WRITE_POLICY), the policy must be signed.
> 
> > These tests assume the policy does not need to be signed.

> Is it a good idea to expect that policy must be signed also for older kernels
> (kernels before 4.5)?

The ability to sign the policy file was introduced with commit 7429b09
("ima: load policy using path").  According to "git branch --
contains", it was upstreamed in linux-4.6.

Mimi


^ permalink raw reply	[flat|nested] 31+ messages in thread

end of thread, other threads:[~2018-04-11 20:03 UTC | newest]

Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-14 15:57 [RFC PATCH v2 0/4] Rewrite tests into new API + fixes Petr Vorel
2018-03-14 15:57 ` [LTP] " Petr Vorel
2018-03-14 15:57 ` [RFC PATCH v2 1/4] security/ima: " Petr Vorel
2018-03-14 15:57   ` [LTP] " Petr Vorel
2018-03-14 16:32   ` Petr Vorel
2018-03-14 16:32     ` [LTP] " Petr Vorel
2018-03-27 19:12   ` Mimi Zohar
2018-03-27 19:12     ` [LTP] " Mimi Zohar
2018-03-29  8:59     ` Petr Vorel
2018-03-29  8:59       ` [LTP] " Petr Vorel
2018-04-10 15:56       ` Mimi Zohar
2018-04-10 15:56         ` [LTP] " Mimi Zohar
2018-04-11 19:03         ` Petr Vorel
2018-04-11 19:03           ` [LTP] " Petr Vorel
2018-04-11 20:03           ` Mimi Zohar
2018-04-11 20:03             ` [LTP] " Mimi Zohar
2018-03-14 15:57 ` [RFC PATCH v2 2/4] security/ima: Run measurements after policy Petr Vorel
2018-03-14 15:57   ` [LTP] " Petr Vorel
2018-03-14 15:57 ` [RFC PATCH v2 3/4] ima/ima_boot_aggregate: Increase MAX_EVENT_SIZE to 8k Petr Vorel
2018-03-14 15:57   ` [LTP] " Petr Vorel
2018-03-27 19:44   ` Mimi Zohar
2018-03-27 19:44     ` [LTP] " Mimi Zohar
2018-03-27 22:23     ` George Wilson
2018-03-29  6:18       ` Petr Vorel
2018-03-29  6:18         ` [LTP] " Petr Vorel
2018-03-14 15:57 ` [RFC PATCH v2 4/4] ima/tpm: Various fixes Petr Vorel
2018-03-14 15:57   ` [LTP] " Petr Vorel
2018-03-26 22:31 ` [RFC PATCH v2 0/4] Rewrite tests into new API + fixes Mimi Zohar
2018-03-26 22:31   ` [LTP] " Mimi Zohar
2018-03-27  9:22   ` Petr Vorel
2018-03-27  9:22     ` [LTP] " Petr Vorel

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.