All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5 0/4] TPM 2.0 fixes in IMA tests
@ 2020-12-14 22:19 ` Petr Vorel
  0 siblings, 0 replies; 30+ messages in thread
From: Petr Vorel @ 2020-12-14 22:19 UTC (permalink / raw)
  To: ltp
  Cc: Petr Vorel, Mimi Zohar, Lakshmi Ramasubramanian, Tushar Sugandhi,
	linux-integrity

Hi Mimi, Lakshmi, Tushar,

sending hopefully a final version. This version was done with big help
from Mimi. Mimi, thank you for your help with this!
I'd like to merge it this week and move on for your other IMA patches
(dm-crypt and SELinux).

Could you please test this, specially on TPM 2.0?
I tested it on tpm_tis MSFT0101:00: (Infineon 9665), which does not
export /sys/kernel/security/tpm0/binary_bios_measurements, but
reading PCR with tsspcrread works.

The only problem which bothers me is failure on ima_policy=tcb:

evmctl ima_measurement /sys/kernel/security/integrity/ima/binary_runtime_measurements -vv
...
sha256: PCRAgg  10: c19866f10132282d4cf20ca45f50078db843f95dc8d1ea8819d0e240cdf3b21c
sha256: TPM PCR-10: df913daa0437a2365f710f6d93a4f2d37146414425d9aaa60740dc635d187158
sha256: PCRAgg 10 does not match TPM PCR-10
Failed to match per TPM bank or SHA1 padded TPM digest(s) (count 1446)
errno: No such file or directory (2)

Thus test get failure for the fist run without --ignore-violations
...
ima_tpm 1 TINFO: using command: evmctl ima_boot_aggregate -v
Using tss2-rc-decode to read PCRs.
ima_tpm 1 TINFO: IMA boot aggregate: '0756853d9378ff6473966e20610a8d1cb97e4dc613cb87adf5e870c8eb93fd0f'
ima_tpm 1 TPASS: bios boot aggregate matches IMA boot aggregate
ima_tpm 2 TINFO: verify PCR values
ima_tpm 2 TINFO: real PCR-10: '6d8aec6291c0c19efdee50e20899939135be073cd4d6e9063e53386f54f9487d'
ima_tpm 2 TFAIL: evmctl failed, trying with --ignore-violations
ima_tpm 2 TINFO: aggregate PCR-10: '6d8aec6291c0c19efdee50e20899939135be073cd4d6e9063e53386f54f9487d'
ima_tpm 2 TPASS: aggregate PCR value matches real PCR value
ima_tpm 3 TINFO: AppArmor enabled, this may affect test results
ima_tpm 3 TINFO: it can be disabled with TST_DISABLE_APPARMOR=1 (requires super/root)
ima_tpm 3 TINFO: loaded AppArmor profiles: none

Summary:
passed   2
failed   1
skipped  0
warnings 0

IMHO unless this is specific for this particular TPM we should skip test
if ima_policy=tcb.

I tried LetsTrust TPM 2.0 for raspberry-pi (Infineon SLB9670, connected
over SPI), but that got even worse - TPM is registered after IMA, thus
unusable).

I'd also like you other IMA tests (dm-crypt and SELinux) before LTP release
(sometimes in January), but due summer vacation we have basically just
this week and maybe first week and maybe first week in January.

Changes v4->v5:
* improved TPM 2.0 detection (e.g. check for /dev/tpmrm0 and /dev/tpm0)
* test2: if evmctl ima_measurement fails, run again with --ignore-violations
* test2: assume TPM 2, if not detected
* print TPM kernel config
* cleanup

Kind regards,
Petr

Petr Vorel (4):
  IMA: Move get_algorithm_digest(), set_digest_index() to ima_setup.sh
  IMA: Rewrite ima_boot_aggregate.c to new API
  ima_tpm.sh: Fix calculating boot aggregate
  ima_tpm.sh: Fix calculating PCR aggregate

 .../integrity/ima/src/ima_boot_aggregate.c    | 114 +++---
 .../integrity/ima/tests/ima_measurements.sh   |  62 +---
 .../security/integrity/ima/tests/ima_setup.sh |  84 ++++-
 .../security/integrity/ima/tests/ima_tpm.sh   | 334 +++++++++++++++---
 4 files changed, 422 insertions(+), 172 deletions(-)

-- 
2.29.2


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

* [LTP] [PATCH v5 0/4] TPM 2.0 fixes in IMA tests
@ 2020-12-14 22:19 ` Petr Vorel
  0 siblings, 0 replies; 30+ messages in thread
From: Petr Vorel @ 2020-12-14 22:19 UTC (permalink / raw)
  To: ltp

Hi Mimi, Lakshmi, Tushar,

sending hopefully a final version. This version was done with big help
from Mimi. Mimi, thank you for your help with this!
I'd like to merge it this week and move on for your other IMA patches
(dm-crypt and SELinux).

Could you please test this, specially on TPM 2.0?
I tested it on tpm_tis MSFT0101:00: (Infineon 9665), which does not
export /sys/kernel/security/tpm0/binary_bios_measurements, but
reading PCR with tsspcrread works.

The only problem which bothers me is failure on ima_policy=tcb:

evmctl ima_measurement /sys/kernel/security/integrity/ima/binary_runtime_measurements -vv
...
sha256: PCRAgg  10: c19866f10132282d4cf20ca45f50078db843f95dc8d1ea8819d0e240cdf3b21c
sha256: TPM PCR-10: df913daa0437a2365f710f6d93a4f2d37146414425d9aaa60740dc635d187158
sha256: PCRAgg 10 does not match TPM PCR-10
Failed to match per TPM bank or SHA1 padded TPM digest(s) (count 1446)
errno: No such file or directory (2)

Thus test get failure for the fist run without --ignore-violations
...
ima_tpm 1 TINFO: using command: evmctl ima_boot_aggregate -v
Using tss2-rc-decode to read PCRs.
ima_tpm 1 TINFO: IMA boot aggregate: '0756853d9378ff6473966e20610a8d1cb97e4dc613cb87adf5e870c8eb93fd0f'
ima_tpm 1 TPASS: bios boot aggregate matches IMA boot aggregate
ima_tpm 2 TINFO: verify PCR values
ima_tpm 2 TINFO: real PCR-10: '6d8aec6291c0c19efdee50e20899939135be073cd4d6e9063e53386f54f9487d'
ima_tpm 2 TFAIL: evmctl failed, trying with --ignore-violations
ima_tpm 2 TINFO: aggregate PCR-10: '6d8aec6291c0c19efdee50e20899939135be073cd4d6e9063e53386f54f9487d'
ima_tpm 2 TPASS: aggregate PCR value matches real PCR value
ima_tpm 3 TINFO: AppArmor enabled, this may affect test results
ima_tpm 3 TINFO: it can be disabled with TST_DISABLE_APPARMOR=1 (requires super/root)
ima_tpm 3 TINFO: loaded AppArmor profiles: none

Summary:
passed   2
failed   1
skipped  0
warnings 0

IMHO unless this is specific for this particular TPM we should skip test
if ima_policy=tcb.

I tried LetsTrust TPM 2.0 for raspberry-pi (Infineon SLB9670, connected
over SPI), but that got even worse - TPM is registered after IMA, thus
unusable).

I'd also like you other IMA tests (dm-crypt and SELinux) before LTP release
(sometimes in January), but due summer vacation we have basically just
this week and maybe first week and maybe first week in January.

Changes v4->v5:
* improved TPM 2.0 detection (e.g. check for /dev/tpmrm0 and /dev/tpm0)
* test2: if evmctl ima_measurement fails, run again with --ignore-violations
* test2: assume TPM 2, if not detected
* print TPM kernel config
* cleanup

Kind regards,
Petr

Petr Vorel (4):
  IMA: Move get_algorithm_digest(), set_digest_index() to ima_setup.sh
  IMA: Rewrite ima_boot_aggregate.c to new API
  ima_tpm.sh: Fix calculating boot aggregate
  ima_tpm.sh: Fix calculating PCR aggregate

 .../integrity/ima/src/ima_boot_aggregate.c    | 114 +++---
 .../integrity/ima/tests/ima_measurements.sh   |  62 +---
 .../security/integrity/ima/tests/ima_setup.sh |  84 ++++-
 .../security/integrity/ima/tests/ima_tpm.sh   | 334 +++++++++++++++---
 4 files changed, 422 insertions(+), 172 deletions(-)

-- 
2.29.2


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

* [PATCH v5 1/4] IMA: Move get_algorithm_digest(), set_digest_index() to ima_setup.sh
  2020-12-14 22:19 ` [LTP] " Petr Vorel
@ 2020-12-14 22:19   ` Petr Vorel
  -1 siblings, 0 replies; 30+ messages in thread
From: Petr Vorel @ 2020-12-14 22:19 UTC (permalink / raw)
  To: ltp
  Cc: Petr Vorel, Mimi Zohar, Lakshmi Ramasubramanian, Tushar Sugandhi,
	linux-integrity

To be reusable by more tests (preparation for next commit).

Call set_digest_index() inside get_algorithm_digest() if needed
instead of expecting get_algorithm_digest() caller to call
set_digest_index() before.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 .../integrity/ima/tests/ima_measurements.sh   | 62 ++--------------
 .../security/integrity/ima/tests/ima_setup.sh | 70 +++++++++++++++++++
 2 files changed, 76 insertions(+), 56 deletions(-)

diff --git a/testcases/kernel/security/integrity/ima/tests/ima_measurements.sh b/testcases/kernel/security/integrity/ima/tests/ima_measurements.sh
index 9a7500c76..1927e937c 100755
--- a/testcases/kernel/security/integrity/ima/tests/ima_measurements.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_measurements.sh
@@ -6,7 +6,7 @@
 #
 # Verify that measurements are added to the measurement list based on policy.
 
-TST_NEEDS_CMDS="awk cut"
+TST_NEEDS_CMDS="awk cut sed"
 TST_SETUP="setup"
 TST_CNT=3
 TST_NEEDS_DEVICE=1
@@ -20,72 +20,22 @@ setup()
 	TEST_FILE="$PWD/test.txt"
 	POLICY="$IMA_DIR/policy"
 	[ -f "$POLICY" ] || tst_res TINFO "not using default policy"
-	DIGEST_INDEX=
-
-	local template="$(tail -1 $ASCII_MEASUREMENTS | cut -d' ' -f 3)"
-	local i
-
-	# parse digest index
-	# https://www.kernel.org/doc/html/latest/security/IMA-templates.html#use
-	case "$template" in
-	ima|ima-ng|ima-sig|ima-buf) DIGEST_INDEX=4 ;;
-	*)
-		# using ima_template_fmt kernel parameter
-		local IFS="|"
-		i=4
-		for word in $template; do
-			if [ "$word" = 'd' -o "$word" = 'd-ng' ]; then
-				DIGEST_INDEX=$i
-				break
-			fi
-			i=$((i+1))
-		done
-	esac
-
-	[ -z "$DIGEST_INDEX" ] && tst_brk TCONF \
-		"Cannot find digest index (template: '$template')"
 }
 
 ima_check()
 {
-	local delimiter=':'
-	local algorithm digest expected_digest line
+	local algorithm digest expected_digest line tmp
 
 	# need to read file to get updated $ASCII_MEASUREMENTS
 	cat $TEST_FILE > /dev/null
 
 	line="$(grep $TEST_FILE $ASCII_MEASUREMENTS | tail -1)"
-	if [ -z "$line" ]; then
-		tst_res TFAIL "cannot find measurement record for '$TEST_FILE'"
-		return
-	fi
-	tst_res TINFO "measurement record: '$line'"
 
-	digest=$(echo "$line" | cut -d' ' -f $DIGEST_INDEX)
-	if [ -z "$digest" ]; then
-		tst_res TFAIL "cannot find digest (index: $DIGEST_INDEX)"
-		return
-	fi
-
-	if [ "${digest#*$delimiter}" != "$digest" ]; then
-		algorithm=$(echo "$digest" | cut -d $delimiter -f 1)
-		digest=$(echo "$digest" | cut -d $delimiter -f 2)
+	if tmp=$(get_algorithm_digest "$line"); then
+		algorithm=$(echo "$tmp" | cut -d'|' -f1)
+		digest=$(echo "$tmp" | cut -d'|' -f2)
 	else
-		case "${#digest}" in
-		32) algorithm="md5" ;;
-		40) algorithm="sha1" ;;
-		*)
-			tst_res TFAIL "algorithm must be either md5 or sha1 (digest: '$digest')"
-			return ;;
-		esac
-	fi
-	if [ -z "$algorithm" ]; then
-		tst_res TFAIL "cannot find algorithm"
-		return
-	fi
-	if [ -z "$digest" ]; then
-		tst_res TFAIL "cannot find digest"
-		return
+		tst_res TBROK "failed to get algorithm/digest for '$TEST_FILE': $tmp"
 	fi
 
 	tst_res TINFO "computing digest for $algorithm algorithm"
diff --git a/testcases/kernel/security/integrity/ima/tests/ima_setup.sh b/testcases/kernel/security/integrity/ima/tests/ima_setup.sh
index eb0087b27..dbf8a1db4 100644
--- a/testcases/kernel/security/integrity/ima/tests/ima_setup.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_setup.sh
@@ -191,6 +191,76 @@ ima_cleanup()
 	fi
 }
 
+set_digest_index()
+{
+	DIGEST_INDEX=
+
+	local template="$(tail -1 $ASCII_MEASUREMENTS | cut -d' ' -f 3)"
+	local i word
+
+	# parse digest index
+	# https://www.kernel.org/doc/html/latest/security/IMA-templates.html#use
+	case "$template" in
+	ima|ima-ng|ima-sig) DIGEST_INDEX=4 ;;
+	*)
+		# using ima_template_fmt kernel parameter
+		local IFS="|"
+		i=4
+		for word in $template; do
+			if [ "$word" = 'd' -o "$word" = 'd-ng' ]; then
+				DIGEST_INDEX=$i
+				break
+			fi
+			i=$((i+1))
+		done
+	esac
+
+	[ -z "$DIGEST_INDEX" ] && tst_brk TCONF \
+		"Cannot find digest index (template: '$template')"
+}
+
+get_algorithm_digest()
+{
+	local line="$1"
+	local delimiter=':'
+	local algorithm digest
+
+	if [ -z "$line" ]; then
+		echo "measurement record not found"
+		return 1
+	fi
+
+	[ -z "$DIGEST_INDEX" ] && set_digest_index
+	digest=$(echo "$line" | cut -d' ' -f $DIGEST_INDEX)
+	if [ -z "$digest" ]; then
+		echo "digest not found (index: $DIGEST_INDEX, line: '$line')"
+		return 1
+	fi
+
+	if [ "${digest#*$delimiter}" != "$digest" ]; then
+		algorithm=$(echo "$digest" | cut -d $delimiter -f 1)
+		digest=$(echo "$digest" | cut -d $delimiter -f 2)
+	else
+		case "${#digest}" in
+		32) algorithm="md5" ;;
+		40) algorithm="sha1" ;;
+		*)
+			echo "algorithm must be either md5 or sha1 (digest: '$digest')"
+			return 1 ;;
+		esac
+	fi
+	if [ -z "$algorithm" ]; then
+		echo "algorithm not found"
+		return 1
+	fi
+	if [ -z "$digest" ]; then
+		echo "digest not found"
+		return 1
+	fi
+
+	echo "$algorithm|$digest"
+}
+
 # loop device is needed to use only for tmpfs
 TMPDIR="${TMPDIR:-/tmp}"
 if [ "$(df -T $TMPDIR | tail -1 | awk '{print $2}')" != "tmpfs" -a -n "$TST_NEEDS_DEVICE" ]; then
-- 
2.29.2


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

* [LTP] [PATCH v5 1/4] IMA: Move get_algorithm_digest(), set_digest_index() to ima_setup.sh
@ 2020-12-14 22:19   ` Petr Vorel
  0 siblings, 0 replies; 30+ messages in thread
From: Petr Vorel @ 2020-12-14 22:19 UTC (permalink / raw)
  To: ltp

To be reusable by more tests (preparation for next commit).

Call set_digest_index() inside get_algorithm_digest() if needed
instead of expecting get_algorithm_digest() caller to call
set_digest_index() before.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 .../integrity/ima/tests/ima_measurements.sh   | 62 ++--------------
 .../security/integrity/ima/tests/ima_setup.sh | 70 +++++++++++++++++++
 2 files changed, 76 insertions(+), 56 deletions(-)

diff --git a/testcases/kernel/security/integrity/ima/tests/ima_measurements.sh b/testcases/kernel/security/integrity/ima/tests/ima_measurements.sh
index 9a7500c76..1927e937c 100755
--- a/testcases/kernel/security/integrity/ima/tests/ima_measurements.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_measurements.sh
@@ -6,7 +6,7 @@
 #
 # Verify that measurements are added to the measurement list based on policy.
 
-TST_NEEDS_CMDS="awk cut"
+TST_NEEDS_CMDS="awk cut sed"
 TST_SETUP="setup"
 TST_CNT=3
 TST_NEEDS_DEVICE=1
@@ -20,72 +20,22 @@ setup()
 	TEST_FILE="$PWD/test.txt"
 	POLICY="$IMA_DIR/policy"
 	[ -f "$POLICY" ] || tst_res TINFO "not using default policy"
-	DIGEST_INDEX=
-
-	local template="$(tail -1 $ASCII_MEASUREMENTS | cut -d' ' -f 3)"
-	local i
-
-	# parse digest index
-	# https://www.kernel.org/doc/html/latest/security/IMA-templates.html#use
-	case "$template" in
-	ima|ima-ng|ima-sig|ima-buf) DIGEST_INDEX=4 ;;
-	*)
-		# using ima_template_fmt kernel parameter
-		local IFS="|"
-		i=4
-		for word in $template; do
-			if [ "$word" = 'd' -o "$word" = 'd-ng' ]; then
-				DIGEST_INDEX=$i
-				break
-			fi
-			i=$((i+1))
-		done
-	esac
-
-	[ -z "$DIGEST_INDEX" ] && tst_brk TCONF \
-		"Cannot find digest index (template: '$template')"
 }
 
 ima_check()
 {
-	local delimiter=':'
-	local algorithm digest expected_digest line
+	local algorithm digest expected_digest line tmp
 
 	# need to read file to get updated $ASCII_MEASUREMENTS
 	cat $TEST_FILE > /dev/null
 
 	line="$(grep $TEST_FILE $ASCII_MEASUREMENTS | tail -1)"
-	if [ -z "$line" ]; then
-		tst_res TFAIL "cannot find measurement record for '$TEST_FILE'"
-		return
-	fi
-	tst_res TINFO "measurement record: '$line'"
 
-	digest=$(echo "$line" | cut -d' ' -f $DIGEST_INDEX)
-	if [ -z "$digest" ]; then
-		tst_res TFAIL "cannot find digest (index: $DIGEST_INDEX)"
-		return
-	fi
-
-	if [ "${digest#*$delimiter}" != "$digest" ]; then
-		algorithm=$(echo "$digest" | cut -d $delimiter -f 1)
-		digest=$(echo "$digest" | cut -d $delimiter -f 2)
+	if tmp=$(get_algorithm_digest "$line"); then
+		algorithm=$(echo "$tmp" | cut -d'|' -f1)
+		digest=$(echo "$tmp" | cut -d'|' -f2)
 	else
-		case "${#digest}" in
-		32) algorithm="md5" ;;
-		40) algorithm="sha1" ;;
-		*)
-			tst_res TFAIL "algorithm must be either md5 or sha1 (digest: '$digest')"
-			return ;;
-		esac
-	fi
-	if [ -z "$algorithm" ]; then
-		tst_res TFAIL "cannot find algorithm"
-		return
-	fi
-	if [ -z "$digest" ]; then
-		tst_res TFAIL "cannot find digest"
-		return
+		tst_res TBROK "failed to get algorithm/digest for '$TEST_FILE': $tmp"
 	fi
 
 	tst_res TINFO "computing digest for $algorithm algorithm"
diff --git a/testcases/kernel/security/integrity/ima/tests/ima_setup.sh b/testcases/kernel/security/integrity/ima/tests/ima_setup.sh
index eb0087b27..dbf8a1db4 100644
--- a/testcases/kernel/security/integrity/ima/tests/ima_setup.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_setup.sh
@@ -191,6 +191,76 @@ ima_cleanup()
 	fi
 }
 
+set_digest_index()
+{
+	DIGEST_INDEX=
+
+	local template="$(tail -1 $ASCII_MEASUREMENTS | cut -d' ' -f 3)"
+	local i word
+
+	# parse digest index
+	# https://www.kernel.org/doc/html/latest/security/IMA-templates.html#use
+	case "$template" in
+	ima|ima-ng|ima-sig) DIGEST_INDEX=4 ;;
+	*)
+		# using ima_template_fmt kernel parameter
+		local IFS="|"
+		i=4
+		for word in $template; do
+			if [ "$word" = 'd' -o "$word" = 'd-ng' ]; then
+				DIGEST_INDEX=$i
+				break
+			fi
+			i=$((i+1))
+		done
+	esac
+
+	[ -z "$DIGEST_INDEX" ] && tst_brk TCONF \
+		"Cannot find digest index (template: '$template')"
+}
+
+get_algorithm_digest()
+{
+	local line="$1"
+	local delimiter=':'
+	local algorithm digest
+
+	if [ -z "$line" ]; then
+		echo "measurement record not found"
+		return 1
+	fi
+
+	[ -z "$DIGEST_INDEX" ] && set_digest_index
+	digest=$(echo "$line" | cut -d' ' -f $DIGEST_INDEX)
+	if [ -z "$digest" ]; then
+		echo "digest not found (index: $DIGEST_INDEX, line: '$line')"
+		return 1
+	fi
+
+	if [ "${digest#*$delimiter}" != "$digest" ]; then
+		algorithm=$(echo "$digest" | cut -d $delimiter -f 1)
+		digest=$(echo "$digest" | cut -d $delimiter -f 2)
+	else
+		case "${#digest}" in
+		32) algorithm="md5" ;;
+		40) algorithm="sha1" ;;
+		*)
+			echo "algorithm must be either md5 or sha1 (digest: '$digest')"
+			return 1 ;;
+		esac
+	fi
+	if [ -z "$algorithm" ]; then
+		echo "algorithm not found"
+		return 1
+	fi
+	if [ -z "$digest" ]; then
+		echo "digest not found"
+		return 1
+	fi
+
+	echo "$algorithm|$digest"
+}
+
 # loop device is needed to use only for tmpfs
 TMPDIR="${TMPDIR:-/tmp}"
 if [ "$(df -T $TMPDIR | tail -1 | awk '{print $2}')" != "tmpfs" -a -n "$TST_NEEDS_DEVICE" ]; then
-- 
2.29.2


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

* [PATCH v5 2/4] IMA: Rewrite ima_boot_aggregate.c to new API
  2020-12-14 22:19 ` [LTP] " Petr Vorel
@ 2020-12-14 22:19   ` Petr Vorel
  -1 siblings, 0 replies; 30+ messages in thread
From: Petr Vorel @ 2020-12-14 22:19 UTC (permalink / raw)
  To: ltp
  Cc: Petr Vorel, Mimi Zohar, Lakshmi Ramasubramanian, Tushar Sugandhi,
	linux-integrity

The main reason was to see TCONF messages, which are printed into stderr
in new API (but to stdout in legacy API) and thus visible as the output
is redirected into the variable.

Changing boot_aggregate: to sha1: to be compatible with evmctl
ima_boot_aggregate.

Also require root (needed for reading
/sys/kernel/security/tpm0/binary_bios_measurements).

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 .../integrity/ima/src/ima_boot_aggregate.c    | 114 +++++++++---------
 .../security/integrity/ima/tests/ima_tpm.sh   |   2 +-
 2 files changed, 58 insertions(+), 58 deletions(-)

diff --git a/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c b/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
index 98893b99a..04d106662 100644
--- a/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
+++ b/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
@@ -1,19 +1,15 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
-* Copyright (c) International Business Machines  Corp., 2009
-*
-* Authors:
-* 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_boot_aggregate.c
-*
-* Calculate a SHA1 boot aggregate value based on the TPM
-* binary_bios_measurements.
-*/
+ * Copyright (c) International Business Machines  Corp., 2009
+ * Copyright (c) 2016-2019 Petr Vorel <pvorel@suse.cz>
+ *
+ * Authors: Mimi Zohar <zohar@us.ibm.com>
+ *
+ * Calculate a SHA1 boot aggregate value based on the TPM 1.2
+ * binary_bios_measurements.
+ */
+
+#include "config.h"
 #include <stdio.h>
 #include <stdlib.h>
 #include <sys/types.h>
@@ -23,10 +19,8 @@
 #include <unistd.h>
 #include <limits.h>
 
-#include "config.h"
-#include "test.h"
-
-char *TCID = "ima_boot_aggregate";
+#include "tst_test.h"
+#include "tst_safe_stdio.h"
 
 #if HAVE_LIBCRYPTO
 #include <openssl/sha.h>
@@ -36,7 +30,24 @@ char *TCID = "ima_boot_aggregate";
 #define MAX_EVENT_DATA_SIZE (MAX_EVENT_SIZE - EVENT_HEADER_SIZE)
 #define NUM_PCRS 8		/*  PCR registers 0-7 in boot aggregate */
 
-int TST_TOTAL = 1;
+static char *debug;
+static char *file;
+
+static unsigned char boot_aggregate[SHA_DIGEST_LENGTH];
+
+static struct {
+	struct {
+		u_int32_t pcr;
+		u_int32_t type;
+		u_int8_t digest[SHA_DIGEST_LENGTH];
+		u_int32_t len;
+	} header __attribute__ ((packed));
+	char *data;
+} event;
+
+static struct {
+	unsigned char digest[SHA_DIGEST_LENGTH];
+} pcr[NUM_PCRS];
 
 static void display_sha1_digest(unsigned char *pcr)
 {
@@ -47,45 +58,24 @@ static void display_sha1_digest(unsigned char *pcr)
 	printf("\n");
 }
 
-int main(int argc, char *argv[])
+static void do_test(void)
 {
-	unsigned char boot_aggregate[SHA_DIGEST_LENGTH];
-	struct {
-		struct {
-			u_int32_t pcr;
-			u_int32_t type;
-			u_int8_t digest[SHA_DIGEST_LENGTH];
-			u_int32_t len;
-		} header __attribute__ ((packed));
-		char *data;
-	} event;
-	struct {
-		unsigned char digest[SHA_DIGEST_LENGTH];
-	} pcr[NUM_PCRS];
 	FILE *fp;
-	int i;
-	int debug = 0;
 	SHA_CTX c;
+	int i;
 
-	if (argc != 2) {
-		printf("format: %s binary_bios_measurement file\n", argv[0]);
-		return 1;
-	}
-	fp = fopen(argv[1], "r");
-	if (!fp) {
-		perror("unable to open pcr file\n");
-		return 1;
-	}
+	if (!file)
+		tst_brk(TBROK, "missing binary_bios_measurement file, specify with -f");
+
+	fp = SAFE_FOPEN(file, "r");
 
 	/* Initialize psuedo PCR registers 0 - 7 */
 	for (i = 0; i < NUM_PCRS; i++)
 		memset(&pcr[i].digest, 0, SHA_DIGEST_LENGTH);
 
 	event.data = malloc(MAX_EVENT_DATA_SIZE);
-	if (!event.data) {
-		printf("Cannot allocate memory\n");
-		return 1;
-	}
+	if (!event.data)
+		tst_brk(TBROK, "cannot allocate memory");
 
 	/* Extend the pseudo PCRs with the event digest */
 	while (fread(&event, sizeof(event.header), 1, fp)) {
@@ -105,13 +95,14 @@ int main(int argc, char *argv[])
 
 #if MAX_EVENT_DATA_SIZE < USHRT_MAX
 		if (event.header.len > MAX_EVENT_DATA_SIZE) {
-			printf("Error event too long\n");
+			tst_res(TWARN, "error event too long");
 			break;
 		}
 #endif
 		fread(event.data, event.header.len, 1, fp);
 	}
-	fclose(fp);
+
+	SAFE_FCLOSE(fp);
 	free(event.data);
 
 	/* Extend the boot aggregate with the pseudo PCR digest values */
@@ -126,14 +117,23 @@ int main(int argc, char *argv[])
 	}
 	SHA1_Final(boot_aggregate, &c);
 
-	printf("boot_aggregate:");
+	printf("sha1:");
 	display_sha1_digest(boot_aggregate);
-	tst_exit();
+	tst_res(TPASS, "found sha1 hash");
 }
 
+static struct tst_option options[] = {
+	{"d", &debug, "-d       enable debug"},
+	{"f:", &file, "-f x     binary_bios_measurement file (required)\n"},
+	{NULL, NULL, NULL}
+};
+
+static struct tst_test test = {
+	.needs_root = 1,
+	.test_all = do_test,
+	.options = options,
+};
+
 #else
-int main(void)
-{
-	tst_brkm(TCONF, NULL, "test requires libcrypto and openssl development packages");
-}
+TST_TEST_TCONF("libcrypto and openssl development packages required");
 #endif
diff --git a/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh b/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
index c69f891f1..dc958eb5c 100755
--- a/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
@@ -33,7 +33,7 @@ test1()
 			tst_res TFAIL "bios boot aggregate is not 0"
 		fi
 	else
-		boot_aggregate=$(ima_boot_aggregate $tpm_bios | grep "boot_aggregate:" | cut -d':' -f2)
+		boot_aggregate=$(ima_boot_aggregate -f $tpm_bios | grep "sha1:" | cut -d':' -f2)
 		if [ "$boot_hash" = "$boot_aggregate" ]; then
 			tst_res TPASS "bios aggregate matches IMA boot aggregate"
 		else
-- 
2.29.2


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

* [LTP] [PATCH v5 2/4] IMA: Rewrite ima_boot_aggregate.c to new API
@ 2020-12-14 22:19   ` Petr Vorel
  0 siblings, 0 replies; 30+ messages in thread
From: Petr Vorel @ 2020-12-14 22:19 UTC (permalink / raw)
  To: ltp

The main reason was to see TCONF messages, which are printed into stderr
in new API (but to stdout in legacy API) and thus visible as the output
is redirected into the variable.

Changing boot_aggregate: to sha1: to be compatible with evmctl
ima_boot_aggregate.

Also require root (needed for reading
/sys/kernel/security/tpm0/binary_bios_measurements).

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 .../integrity/ima/src/ima_boot_aggregate.c    | 114 +++++++++---------
 .../security/integrity/ima/tests/ima_tpm.sh   |   2 +-
 2 files changed, 58 insertions(+), 58 deletions(-)

diff --git a/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c b/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
index 98893b99a..04d106662 100644
--- a/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
+++ b/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
@@ -1,19 +1,15 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
-* Copyright (c) International Business Machines  Corp., 2009
-*
-* Authors:
-* 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_boot_aggregate.c
-*
-* Calculate a SHA1 boot aggregate value based on the TPM
-* binary_bios_measurements.
-*/
+ * Copyright (c) International Business Machines  Corp., 2009
+ * Copyright (c) 2016-2019 Petr Vorel <pvorel@suse.cz>
+ *
+ * Authors: Mimi Zohar <zohar@us.ibm.com>
+ *
+ * Calculate a SHA1 boot aggregate value based on the TPM 1.2
+ * binary_bios_measurements.
+ */
+
+#include "config.h"
 #include <stdio.h>
 #include <stdlib.h>
 #include <sys/types.h>
@@ -23,10 +19,8 @@
 #include <unistd.h>
 #include <limits.h>
 
-#include "config.h"
-#include "test.h"
-
-char *TCID = "ima_boot_aggregate";
+#include "tst_test.h"
+#include "tst_safe_stdio.h"
 
 #if HAVE_LIBCRYPTO
 #include <openssl/sha.h>
@@ -36,7 +30,24 @@ char *TCID = "ima_boot_aggregate";
 #define MAX_EVENT_DATA_SIZE (MAX_EVENT_SIZE - EVENT_HEADER_SIZE)
 #define NUM_PCRS 8		/*  PCR registers 0-7 in boot aggregate */
 
-int TST_TOTAL = 1;
+static char *debug;
+static char *file;
+
+static unsigned char boot_aggregate[SHA_DIGEST_LENGTH];
+
+static struct {
+	struct {
+		u_int32_t pcr;
+		u_int32_t type;
+		u_int8_t digest[SHA_DIGEST_LENGTH];
+		u_int32_t len;
+	} header __attribute__ ((packed));
+	char *data;
+} event;
+
+static struct {
+	unsigned char digest[SHA_DIGEST_LENGTH];
+} pcr[NUM_PCRS];
 
 static void display_sha1_digest(unsigned char *pcr)
 {
@@ -47,45 +58,24 @@ static void display_sha1_digest(unsigned char *pcr)
 	printf("\n");
 }
 
-int main(int argc, char *argv[])
+static void do_test(void)
 {
-	unsigned char boot_aggregate[SHA_DIGEST_LENGTH];
-	struct {
-		struct {
-			u_int32_t pcr;
-			u_int32_t type;
-			u_int8_t digest[SHA_DIGEST_LENGTH];
-			u_int32_t len;
-		} header __attribute__ ((packed));
-		char *data;
-	} event;
-	struct {
-		unsigned char digest[SHA_DIGEST_LENGTH];
-	} pcr[NUM_PCRS];
 	FILE *fp;
-	int i;
-	int debug = 0;
 	SHA_CTX c;
+	int i;
 
-	if (argc != 2) {
-		printf("format: %s binary_bios_measurement file\n", argv[0]);
-		return 1;
-	}
-	fp = fopen(argv[1], "r");
-	if (!fp) {
-		perror("unable to open pcr file\n");
-		return 1;
-	}
+	if (!file)
+		tst_brk(TBROK, "missing binary_bios_measurement file, specify with -f");
+
+	fp = SAFE_FOPEN(file, "r");
 
 	/* Initialize psuedo PCR registers 0 - 7 */
 	for (i = 0; i < NUM_PCRS; i++)
 		memset(&pcr[i].digest, 0, SHA_DIGEST_LENGTH);
 
 	event.data = malloc(MAX_EVENT_DATA_SIZE);
-	if (!event.data) {
-		printf("Cannot allocate memory\n");
-		return 1;
-	}
+	if (!event.data)
+		tst_brk(TBROK, "cannot allocate memory");
 
 	/* Extend the pseudo PCRs with the event digest */
 	while (fread(&event, sizeof(event.header), 1, fp)) {
@@ -105,13 +95,14 @@ int main(int argc, char *argv[])
 
 #if MAX_EVENT_DATA_SIZE < USHRT_MAX
 		if (event.header.len > MAX_EVENT_DATA_SIZE) {
-			printf("Error event too long\n");
+			tst_res(TWARN, "error event too long");
 			break;
 		}
 #endif
 		fread(event.data, event.header.len, 1, fp);
 	}
-	fclose(fp);
+
+	SAFE_FCLOSE(fp);
 	free(event.data);
 
 	/* Extend the boot aggregate with the pseudo PCR digest values */
@@ -126,14 +117,23 @@ int main(int argc, char *argv[])
 	}
 	SHA1_Final(boot_aggregate, &c);
 
-	printf("boot_aggregate:");
+	printf("sha1:");
 	display_sha1_digest(boot_aggregate);
-	tst_exit();
+	tst_res(TPASS, "found sha1 hash");
 }
 
+static struct tst_option options[] = {
+	{"d", &debug, "-d       enable debug"},
+	{"f:", &file, "-f x     binary_bios_measurement file (required)\n"},
+	{NULL, NULL, NULL}
+};
+
+static struct tst_test test = {
+	.needs_root = 1,
+	.test_all = do_test,
+	.options = options,
+};
+
 #else
-int main(void)
-{
-	tst_brkm(TCONF, NULL, "test requires libcrypto and openssl development packages");
-}
+TST_TEST_TCONF("libcrypto and openssl development packages required");
 #endif
diff --git a/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh b/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
index c69f891f1..dc958eb5c 100755
--- a/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
@@ -33,7 +33,7 @@ test1()
 			tst_res TFAIL "bios boot aggregate is not 0"
 		fi
 	else
-		boot_aggregate=$(ima_boot_aggregate $tpm_bios | grep "boot_aggregate:" | cut -d':' -f2)
+		boot_aggregate=$(ima_boot_aggregate -f $tpm_bios | grep "sha1:" | cut -d':' -f2)
 		if [ "$boot_hash" = "$boot_aggregate" ]; then
 			tst_res TPASS "bios aggregate matches IMA boot aggregate"
 		else
-- 
2.29.2


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

* [PATCH v5 3/4] ima_tpm.sh: Fix calculating boot aggregate
  2020-12-14 22:19 ` [LTP] " Petr Vorel
@ 2020-12-14 22:19   ` Petr Vorel
  -1 siblings, 0 replies; 30+ messages in thread
From: Petr Vorel @ 2020-12-14 22:19 UTC (permalink / raw)
  To: ltp
  Cc: Petr Vorel, Mimi Zohar, Lakshmi Ramasubramanian, Tushar Sugandhi,
	linux-integrity, Mimi Zohar

for TPM 2.0 or kernel >= v5.8-rc1:
6f1a1d103b48 ima: ("Switch to ima_hash_algo for boot aggregate")

Test still fails with newer TPM 2.0 on kernel < v5.8-rc1.

Test was failing, because it expect SHA1 (we ignore MD5) hash, but for TPM 2.0
is now used IMA default hash algorithm (by default default SHA256).
This is similar for entries in IMA measurement list so we can reuse
already existing code.

Reading other algorithms than SHA1 or support TPM 2.0 requires evmctl
>= 1.3.1 (1.3 would also work for test1, but will be required for test2).

Although recent evmctl is recommended, to support older kernels and TPMs
which support only SHA1, get boot aggregate with old our legacy
ima_boot_aggregate.c.

Also fixed cases:
* testing with no TPM device:
* TPM TPM 2.0 devices which does not export event log
(/sys/kernel/security/tpm0/binary_bios_measurements).

Also fixed test without TPM device (when IMA TPM-bypass is tested)
as some TPM 2.0 devices does not export event log
(/sys/kernel/security/tpm0/binary_bios_measurements).
This does not require evmctl at all.

Also try best to detect TPM major version (1, 2 or none - assume
TPM-bypass). This fixes testing with TPM 2.0 device which does not
export event log (/sys/kernel/security/tpm0/binary_bios_measurements):
not wrongly assuming TPM-bypass when kernel didn't export other TPM
2.0 files we check in get_tpm_version() but bios boot aggregate is
correct (i.e. not 0x00s). In that case evmctl ima_boot_aggregate can get
boot aggregate even without TPM event log.

Co-developed-by: Mimi Zohar <zohar@linux.ibm.com>
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
Changes v4->v5:
* add .needs_root = 1

 .../security/integrity/ima/tests/ima_tpm.sh   | 176 +++++++++++++++---
 1 file changed, 154 insertions(+), 22 deletions(-)

diff --git a/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh b/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
index dc958eb5c..195fcb16c 100755
--- a/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
@@ -7,39 +7,171 @@
 # Verify the boot and PCR aggregates.
 
 TST_CNT=2
-TST_NEEDS_CMDS="awk cut ima_boot_aggregate"
+TST_NEEDS_CMDS="awk cut"
+TST_SETUP="setup"
 
 . ima_setup.sh
 
-test1()
-{
-	tst_res TINFO "verify boot aggregate"
+EVMCTL_REQUIRED='1.3.1'
+ERRMSG_EVMCTL="=> install evmctl >= $EVMCTL_REQUIRED"
 
-	local zero="0000000000000000000000000000000000000000"
-	local tpm_bios="$SECURITYFS/tpm0/binary_bios_measurements"
-	local ima_measurements="$ASCII_MEASUREMENTS"
-	local boot_aggregate boot_hash line
+setup()
+{
+	local config="${KCONFIG_PATH:-/boot/config-$(uname -r)}"
+	local line tmp
 
-	# IMA boot aggregate
-	read line < $ima_measurements
-	boot_hash=$(echo $line | awk '{print $(NF-1)}' | cut -d':' -f2)
+	read line < $ASCII_MEASUREMENTS
+	if tmp=$(get_algorithm_digest "$line"); then
+		ALGORITHM=$(echo "$tmp" | cut -d'|' -f1)
+		DIGEST=$(echo "$tmp" | cut -d'|' -f2)
+	else
+		tst_brk TBROK "failed to get algorithm/digest: $tmp"
+	fi
+	tst_res TINFO "used algorithm: $ALGORITHM"
 
-	if [ ! -f "$tpm_bios" ]; then
-		tst_res TINFO "TPM Hardware Support not enabled in kernel or no TPM chip found"
+	TPM_VERSION="$(get_tpm_version)"
+	if [ -z "$TPM_VERSION" ]; then
+		tst_res TINFO "TPM hardware support not enabled in kernel or no TPM chip found, testing TPM-bypass"
+	else
+		tst_res TINFO "TMP major version: $TPM_VERSION"
+	fi
 
-		if [ "$boot_hash" = "$zero" ]; then
-			tst_res TPASS "bios boot aggregate is 0"
-		else
-			tst_res TFAIL "bios boot aggregate is not 0"
+	if ! check_evmctl $EVMCTL_REQUIRED; then
+		if [ "$ALGORITHM" != "sha1" ]; then
+			tst_brk TCONF "algorithm not sha1 ($ALGORITHM) $ERRMSG_EVMCTL"
 		fi
+		MISSING_EVMCTL=1
+	fi
+
+	if [ -r "$config" ]; then
+		tst_res TINFO "TPM kernel config:"
+		for i in $(grep -e ^CONFIG_.*_TPM -e ^CONFIG_TCG $config); do
+			tst_res TINFO "$i"
+		done
+	fi
+}
+
+# check_evmctl REQUIRED_TPM_VERSION
+# return: 0: evmctl is new enough, 1: version older than required (or version < v0.9)
+check_evmctl()
+{
+	local required="$1"
+
+	local r1="$(echo $required | cut -d. -f1)"
+	local r2="$(echo $required | cut -d. -f2)"
+	local r3="$(echo $required | cut -d. -f3)"
+	[ -z "$r3" ] && r3=0
+
+	tst_is_int "$r1" || tst_brk TBROK "required major version not int ($v1)"
+	tst_is_int "$r2" || tst_brk TBROK "required minor version not int ($v2)"
+	tst_is_int "$r3" || tst_brk TBROK "required patch version not int ($v3)"
+
+	tst_check_cmds evmctl || return 1
+
+	local v="$(evmctl --version | cut -d' ' -f2)"
+	[ -z "$v" ] && return 1
+	tst_res TINFO "evmctl version: $v"
+
+	local v1="$(echo $v | cut -d. -f1)"
+	local v2="$(echo $v | cut -d. -f2)"
+	local v3="$(echo $v | cut -d. -f3)"
+	[ -z "$v3" ] && v3=0
+
+	if [ $v1 -lt $r1 ] || [ $v1 -eq $r1 -a $v2 -lt $r2 ] || \
+		[ $v1 -eq $r1 -a $v2 -eq $r2 -a $v3 -lt $r3 ]; then
+		return 1
+	fi
+	return 0
+}
+
+# prints major version: 1: TPM 1.2, 2: TPM 2.0
+# or nothing on TPM-bypass (no TPM device)
+# WARNING: Detecting TPM 2.0 can fail due kernel not exporting TPM 2.0 files.
+get_tpm_version()
+{
+	if [ -f /sys/class/tpm/tpm0/tpm_version_major ]; then
+		cat /sys/class/tpm/tpm0/tpm_version_major
+		return
+	fi
+
+	if [ -f /sys/class/tpm/tpm0/device/caps -o \
+		-f /sys/class/misc/tpm0/device/caps ]; then
+		echo 1
+		return
+	fi
+
+	if [ -c /dev/tpmrm0 -a -c /dev/tpm0 ]; then
+		echo 2
+		return
+	fi
+
+	if [ ! -d /sys/class/tpm/tpm0/ -a ! -d /sys/class/misc/tpm0/ ]; then
+		return
+	fi
+
+	tst_require_cmds dmesg
+	if dmesg | grep -q 'activating TPM-bypass'; then
+		return
+	elif dmesg | grep -q '1\.2 TPM (device-id'; then
+		echo 1
+		return
+	elif dmesg | grep -q '2\.0 TPM (device-id'; then
+		echo 2
+		return
+	fi
+}
+
+test1_tpm_bypass_mode()
+{
+	local zero=$(echo $DIGEST | awk '{gsub(/./, "0")}; {print}')
+
+	if [ "$DIGEST" = "$zero" ]; then
+		tst_res TPASS "bios boot aggregate is $zero"
 	else
-		boot_aggregate=$(ima_boot_aggregate -f $tpm_bios | grep "sha1:" | 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"
+		tst_res TFAIL "bios boot aggregate is not $zero ($DIGEST), kernel didn't export TPM 2.0 files for TPM device?"
+		return 1
+	fi
+}
+
+test1_hw_tpm()
+{
+	local tpm_bios="$SECURITYFS/tpm0/binary_bios_measurements"
+	local cmd="evmctl ima_boot_aggregate -v"
+	local boot_aggregate
+
+	[ -z "$TPM_VERSION" ] && \
+		tst_res TWARN "TPM-bypass failed, trying to test TPM device (unknown TPM version)"
+
+	if [ "$MISSING_EVMCTL" = 1 ]; then
+		if [ ! -f "$tpm_bios" ]; then
+			tst_res TCONF "missing $tpm_bios $ERRMSG_EVMCTL"
+			return
 		fi
+		tst_check_cmds ima_boot_aggregate || return
+		cmd="ima_boot_aggregate -f $tpm_bios"
 	fi
+	tst_res TINFO "using command: $cmd"
+
+	boot_aggregate=$($cmd | grep "$ALGORITHM:" | cut -d':' -f2)
+	if [ -z "$boot_aggregate" ]; then
+		tst_res TFAIL "failed to get boot aggregate"
+		return
+	fi
+	tst_res TINFO "IMA boot aggregate: '$boot_aggregate'"
+
+	if [ "$DIGEST" = "$boot_aggregate" ]; then
+		tst_res TPASS "bios boot aggregate matches IMA boot aggregate"
+	else
+		tst_res TFAIL "bios boot aggregate does not match IMA boot aggregate ($DIGEST)"
+	fi
+}
+
+test1()
+{
+	tst_res TINFO "verify boot aggregate"
+
+	# deliberately try test1_hw_tpm() if test1_tpm_bypass_mode() fails
+	[ -z "$TPM_VERSION" ] && test1_tpm_bypass_mode || test1_hw_tpm
 }
 
 # Probably cleaner to programmatically read the PCR values directly
-- 
2.29.2


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

* [LTP] [PATCH v5 3/4] ima_tpm.sh: Fix calculating boot aggregate
@ 2020-12-14 22:19   ` Petr Vorel
  0 siblings, 0 replies; 30+ messages in thread
From: Petr Vorel @ 2020-12-14 22:19 UTC (permalink / raw)
  To: ltp

for TPM 2.0 or kernel >= v5.8-rc1:
6f1a1d103b48 ima: ("Switch to ima_hash_algo for boot aggregate")

Test still fails with newer TPM 2.0 on kernel < v5.8-rc1.

Test was failing, because it expect SHA1 (we ignore MD5) hash, but for TPM 2.0
is now used IMA default hash algorithm (by default default SHA256).
This is similar for entries in IMA measurement list so we can reuse
already existing code.

Reading other algorithms than SHA1 or support TPM 2.0 requires evmctl
>= 1.3.1 (1.3 would also work for test1, but will be required for test2).

Although recent evmctl is recommended, to support older kernels and TPMs
which support only SHA1, get boot aggregate with old our legacy
ima_boot_aggregate.c.

Also fixed cases:
* testing with no TPM device:
* TPM TPM 2.0 devices which does not export event log
(/sys/kernel/security/tpm0/binary_bios_measurements).

Also fixed test without TPM device (when IMA TPM-bypass is tested)
as some TPM 2.0 devices does not export event log
(/sys/kernel/security/tpm0/binary_bios_measurements).
This does not require evmctl at all.

Also try best to detect TPM major version (1, 2 or none - assume
TPM-bypass). This fixes testing with TPM 2.0 device which does not
export event log (/sys/kernel/security/tpm0/binary_bios_measurements):
not wrongly assuming TPM-bypass when kernel didn't export other TPM
2.0 files we check in get_tpm_version() but bios boot aggregate is
correct (i.e. not 0x00s). In that case evmctl ima_boot_aggregate can get
boot aggregate even without TPM event log.

Co-developed-by: Mimi Zohar <zohar@linux.ibm.com>
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
Changes v4->v5:
* add .needs_root = 1

 .../security/integrity/ima/tests/ima_tpm.sh   | 176 +++++++++++++++---
 1 file changed, 154 insertions(+), 22 deletions(-)

diff --git a/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh b/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
index dc958eb5c..195fcb16c 100755
--- a/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
@@ -7,39 +7,171 @@
 # Verify the boot and PCR aggregates.
 
 TST_CNT=2
-TST_NEEDS_CMDS="awk cut ima_boot_aggregate"
+TST_NEEDS_CMDS="awk cut"
+TST_SETUP="setup"
 
 . ima_setup.sh
 
-test1()
-{
-	tst_res TINFO "verify boot aggregate"
+EVMCTL_REQUIRED='1.3.1'
+ERRMSG_EVMCTL="=> install evmctl >= $EVMCTL_REQUIRED"
 
-	local zero="0000000000000000000000000000000000000000"
-	local tpm_bios="$SECURITYFS/tpm0/binary_bios_measurements"
-	local ima_measurements="$ASCII_MEASUREMENTS"
-	local boot_aggregate boot_hash line
+setup()
+{
+	local config="${KCONFIG_PATH:-/boot/config-$(uname -r)}"
+	local line tmp
 
-	# IMA boot aggregate
-	read line < $ima_measurements
-	boot_hash=$(echo $line | awk '{print $(NF-1)}' | cut -d':' -f2)
+	read line < $ASCII_MEASUREMENTS
+	if tmp=$(get_algorithm_digest "$line"); then
+		ALGORITHM=$(echo "$tmp" | cut -d'|' -f1)
+		DIGEST=$(echo "$tmp" | cut -d'|' -f2)
+	else
+		tst_brk TBROK "failed to get algorithm/digest: $tmp"
+	fi
+	tst_res TINFO "used algorithm: $ALGORITHM"
 
-	if [ ! -f "$tpm_bios" ]; then
-		tst_res TINFO "TPM Hardware Support not enabled in kernel or no TPM chip found"
+	TPM_VERSION="$(get_tpm_version)"
+	if [ -z "$TPM_VERSION" ]; then
+		tst_res TINFO "TPM hardware support not enabled in kernel or no TPM chip found, testing TPM-bypass"
+	else
+		tst_res TINFO "TMP major version: $TPM_VERSION"
+	fi
 
-		if [ "$boot_hash" = "$zero" ]; then
-			tst_res TPASS "bios boot aggregate is 0"
-		else
-			tst_res TFAIL "bios boot aggregate is not 0"
+	if ! check_evmctl $EVMCTL_REQUIRED; then
+		if [ "$ALGORITHM" != "sha1" ]; then
+			tst_brk TCONF "algorithm not sha1 ($ALGORITHM) $ERRMSG_EVMCTL"
 		fi
+		MISSING_EVMCTL=1
+	fi
+
+	if [ -r "$config" ]; then
+		tst_res TINFO "TPM kernel config:"
+		for i in $(grep -e ^CONFIG_.*_TPM -e ^CONFIG_TCG $config); do
+			tst_res TINFO "$i"
+		done
+	fi
+}
+
+# check_evmctl REQUIRED_TPM_VERSION
+# return: 0: evmctl is new enough, 1: version older than required (or version < v0.9)
+check_evmctl()
+{
+	local required="$1"
+
+	local r1="$(echo $required | cut -d. -f1)"
+	local r2="$(echo $required | cut -d. -f2)"
+	local r3="$(echo $required | cut -d. -f3)"
+	[ -z "$r3" ] && r3=0
+
+	tst_is_int "$r1" || tst_brk TBROK "required major version not int ($v1)"
+	tst_is_int "$r2" || tst_brk TBROK "required minor version not int ($v2)"
+	tst_is_int "$r3" || tst_brk TBROK "required patch version not int ($v3)"
+
+	tst_check_cmds evmctl || return 1
+
+	local v="$(evmctl --version | cut -d' ' -f2)"
+	[ -z "$v" ] && return 1
+	tst_res TINFO "evmctl version: $v"
+
+	local v1="$(echo $v | cut -d. -f1)"
+	local v2="$(echo $v | cut -d. -f2)"
+	local v3="$(echo $v | cut -d. -f3)"
+	[ -z "$v3" ] && v3=0
+
+	if [ $v1 -lt $r1 ] || [ $v1 -eq $r1 -a $v2 -lt $r2 ] || \
+		[ $v1 -eq $r1 -a $v2 -eq $r2 -a $v3 -lt $r3 ]; then
+		return 1
+	fi
+	return 0
+}
+
+# prints major version: 1: TPM 1.2, 2: TPM 2.0
+# or nothing on TPM-bypass (no TPM device)
+# WARNING: Detecting TPM 2.0 can fail due kernel not exporting TPM 2.0 files.
+get_tpm_version()
+{
+	if [ -f /sys/class/tpm/tpm0/tpm_version_major ]; then
+		cat /sys/class/tpm/tpm0/tpm_version_major
+		return
+	fi
+
+	if [ -f /sys/class/tpm/tpm0/device/caps -o \
+		-f /sys/class/misc/tpm0/device/caps ]; then
+		echo 1
+		return
+	fi
+
+	if [ -c /dev/tpmrm0 -a -c /dev/tpm0 ]; then
+		echo 2
+		return
+	fi
+
+	if [ ! -d /sys/class/tpm/tpm0/ -a ! -d /sys/class/misc/tpm0/ ]; then
+		return
+	fi
+
+	tst_require_cmds dmesg
+	if dmesg | grep -q 'activating TPM-bypass'; then
+		return
+	elif dmesg | grep -q '1\.2 TPM (device-id'; then
+		echo 1
+		return
+	elif dmesg | grep -q '2\.0 TPM (device-id'; then
+		echo 2
+		return
+	fi
+}
+
+test1_tpm_bypass_mode()
+{
+	local zero=$(echo $DIGEST | awk '{gsub(/./, "0")}; {print}')
+
+	if [ "$DIGEST" = "$zero" ]; then
+		tst_res TPASS "bios boot aggregate is $zero"
 	else
-		boot_aggregate=$(ima_boot_aggregate -f $tpm_bios | grep "sha1:" | 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"
+		tst_res TFAIL "bios boot aggregate is not $zero ($DIGEST), kernel didn't export TPM 2.0 files for TPM device?"
+		return 1
+	fi
+}
+
+test1_hw_tpm()
+{
+	local tpm_bios="$SECURITYFS/tpm0/binary_bios_measurements"
+	local cmd="evmctl ima_boot_aggregate -v"
+	local boot_aggregate
+
+	[ -z "$TPM_VERSION" ] && \
+		tst_res TWARN "TPM-bypass failed, trying to test TPM device (unknown TPM version)"
+
+	if [ "$MISSING_EVMCTL" = 1 ]; then
+		if [ ! -f "$tpm_bios" ]; then
+			tst_res TCONF "missing $tpm_bios $ERRMSG_EVMCTL"
+			return
 		fi
+		tst_check_cmds ima_boot_aggregate || return
+		cmd="ima_boot_aggregate -f $tpm_bios"
 	fi
+	tst_res TINFO "using command: $cmd"
+
+	boot_aggregate=$($cmd | grep "$ALGORITHM:" | cut -d':' -f2)
+	if [ -z "$boot_aggregate" ]; then
+		tst_res TFAIL "failed to get boot aggregate"
+		return
+	fi
+	tst_res TINFO "IMA boot aggregate: '$boot_aggregate'"
+
+	if [ "$DIGEST" = "$boot_aggregate" ]; then
+		tst_res TPASS "bios boot aggregate matches IMA boot aggregate"
+	else
+		tst_res TFAIL "bios boot aggregate does not match IMA boot aggregate ($DIGEST)"
+	fi
+}
+
+test1()
+{
+	tst_res TINFO "verify boot aggregate"
+
+	# deliberately try test1_hw_tpm() if test1_tpm_bypass_mode() fails
+	[ -z "$TPM_VERSION" ] && test1_tpm_bypass_mode || test1_hw_tpm
 }
 
 # Probably cleaner to programmatically read the PCR values directly
-- 
2.29.2


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

* [PATCH v5 4/4] ima_tpm.sh: Fix calculating PCR aggregate
  2020-12-14 22:19 ` [LTP] " Petr Vorel
@ 2020-12-14 22:19   ` Petr Vorel
  -1 siblings, 0 replies; 30+ messages in thread
From: Petr Vorel @ 2020-12-14 22:19 UTC (permalink / raw)
  To: ltp
  Cc: Petr Vorel, Mimi Zohar, Lakshmi Ramasubramanian, Tushar Sugandhi,
	linux-integrity, Mimi Zohar

for TPM 2.0 and support more evmctl versions.

Because exporting PCR registers for TPM 2.0 has not been upstreamed [1],
we use user space code, which requires evmctl >= 1.3.1 and tsspcrread.
Using evmctl allows to test for TPM devices which does not export event
log (/sys/kernel/security/tpm0/binary_bios_measurements).

For TPM 1.2 read tpm0 device's pcrs file from sysfs. (tss1pcrread could
be also used, but it's not yet packaged by distros.)

For old kernels which use SHA1/MD5, any evmctl version is required (evmctl
ima_measurement was introduced in very old v0.7), but
* newer sysctl path /sys/class/tpm/tpm0/device/pcrs requires evmctl 1.1
* using ima_policy=tcb requires 1.3.1 due --ignore-violations

We now support output format of ima_measurement command for various
evmctl versions:
* 1.3: "sha256: TPM PCR-10:" (or other algorithm, e.g. "sha1:")
* 1.1-1.2.1: "HW PCR-10:" (the only previously supported format)
* 0.7-1.0: "PCR-10:"

NOTE: we ignore evmctl failure for evmctl < 1.3.1 (missing --ignore-violations,
also evmctl < 1.1 fails with "PCRAgg does not match PCR-10")

As for previous commit fix testing with TPM 2.0 device which does not
export event log (/sys/kernel/security/tpm0/binary_bios_measurements):
not wrongly assuming TPM-bypass when kernel didn't export other TPM
2.0 files we check in get_tpm_version() but bios boot aggregate is
correct (i.e. not 0x00s). In that case evmctl ima_boot_aggregate can get
boot aggregate even without TPM event log.

[1] https://patchwork.kernel.org/patch/11759729/

Co-developed-by: Mimi Zohar <zohar@linux.ibm.com>
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
Changes v4->v5:
* improved TPM 2.0 detection (e.g. check for /dev/tpmrm0 and /dev/tpm0)
* test2: if evmctl ima_measurement fails, run again with --ignore-violations
* test2: assume TPM 2, if not detected
* print TPM kernel config
* cleanup

 .../security/integrity/ima/tests/ima_setup.sh |  14 +-
 .../security/integrity/ima/tests/ima_tpm.sh   | 176 +++++++++++++-----
 2 files changed, 144 insertions(+), 46 deletions(-)

diff --git a/testcases/kernel/security/integrity/ima/tests/ima_setup.sh b/testcases/kernel/security/integrity/ima/tests/ima_setup.sh
index dbf8a1db4..59a7ffeac 100644
--- a/testcases/kernel/security/integrity/ima/tests/ima_setup.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_setup.sh
@@ -93,7 +93,7 @@ require_ima_policy_content()
 	fi
 }
 
-require_ima_policy_cmdline()
+check_ima_policy_cmdline()
 {
 	local policy="$1"
 	local i
@@ -101,10 +101,18 @@ require_ima_policy_cmdline()
 	grep -q "ima_$policy" /proc/cmdline && return
 	for i in $(cat /proc/cmdline); do
 		if echo "$i" | grep -q '^ima_policy='; then
-			echo "$i" | grep -q -e "|[ ]*$policy" -e "$policy[ ]*|" -e "=$policy" && return
+			echo "$i" | grep -q -e "|[ ]*$policy" -e "$policy[ ]*|" -e "=$policy" && return 0
 		fi
 	done
-	tst_brk TCONF "IMA measurement tests require builtin IMA $policy policy (e.g. ima_policy=$policy kernel parameter)"
+	return 1
+}
+
+require_ima_policy_cmdline()
+{
+	local policy="$1"
+
+	check_ima_policy_cmdline $policy || \
+		tst_brk TCONF "IMA measurement tests require builtin IMA $policy policy (e.g. ima_policy=$policy kernel parameter)"
 }
 
 mount_helper()
diff --git a/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh b/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
index 195fcb16c..233fdeed8 100755
--- a/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
@@ -7,13 +7,14 @@
 # Verify the boot and PCR aggregates.
 
 TST_CNT=2
-TST_NEEDS_CMDS="awk cut"
+TST_NEEDS_CMDS="awk cut tail"
 TST_SETUP="setup"
 
 . ima_setup.sh
 
 EVMCTL_REQUIRED='1.3.1'
 ERRMSG_EVMCTL="=> install evmctl >= $EVMCTL_REQUIRED"
+ERRMSG_TPM="TPM hardware support not enabled in kernel or no TPM chip found"
 
 setup()
 {
@@ -31,7 +32,7 @@ setup()
 
 	TPM_VERSION="$(get_tpm_version)"
 	if [ -z "$TPM_VERSION" ]; then
-		tst_res TINFO "TPM hardware support not enabled in kernel or no TPM chip found, testing TPM-bypass"
+		tst_res TINFO "$ERRMSG_TPM, testing TPM-bypass"
 	else
 		tst_res TINFO "TMP major version: $TPM_VERSION"
 	fi
@@ -121,6 +122,93 @@ get_tpm_version()
 	fi
 }
 
+read_pcr_tpm1()
+{
+	local pcrs_path="/sys/class/tpm/tpm0/device/pcrs"
+	local evmctl_required="1.1"
+	local hash pcr
+
+	if [ ! -f "$pcrs_path" ]; then
+		pcrs_path="/sys/class/misc/tpm0/device/pcrs"
+	elif ! check_evmctl $evmctl_required; then
+		echo "evmctl >= $evmctl_required required"
+		return 32
+	fi
+
+	if [ ! -f "$pcrs_path" ]; then
+		echo "missing PCR file $pcrs_path ($ERRMSG_TPM)"
+		return 32
+	fi
+
+	while read line; do
+		pcr="$(echo $line | cut -d':' -f1)"
+		hash="$(echo $line | cut -d':' -f2 | awk '{ gsub (" ", "", $0); print tolower($0) }')"
+		echo "$pcr: $hash"
+	done < $pcrs_path
+
+	return 0
+}
+
+# NOTE: TPM 1.2 would require to use tss1pcrread which is not fully adopted
+# by distros yet.
+read_pcr_tpm2()
+{
+	local pcrmax=23
+	local pcrread="tsspcrread -halg $ALGORITHM"
+	local i pcr
+
+	tst_check_cmds tsspcrread || return 1
+
+	for i in $(seq 0 $pcrmax); do
+		pcr=$($pcrread -ha "$i" -ns)
+		if [ $? -ne 0 ]; then
+			echo "tsspcrread failed: $pcr"
+			return 1
+		fi
+		printf "PCR-%02d: %s\n" $i "$pcr"
+	done
+
+	return 0
+}
+
+get_pcr10_aggregate()
+{
+	local cmd="evmctl -vv ima_measurement $BINARY_MEASUREMENTS"
+	local msg="$ERRMSG_EVMCTL"
+	local res=TCONF
+	local pcr ret
+
+	if [ -z "$MISSING_EVMCTL" ]; then
+		msg=
+		res=TFAIL
+	fi
+
+	$cmd > hash.txt 2>&1
+	ret=$?
+	if [ $ret -ne 0 -a -z "$MISSING_EVMCTL" ]; then
+		tst_res TFAIL "evmctl failed, trying with --ignore-violations"
+		cmd="$cmd --ignore-violations"
+		$cmd > hash.txt 2>&1
+		ret=$?
+	elif [ $ret -ne 0 -a "$MISSING_EVMCTL" = 1 ]; then
+		tst_brk TFAIL "evmctl failed $msg"
+	fi
+
+	[ $ret -ne 0 ] && tst_res TWARN "evmctl failed, trying to continue $msg"
+
+	pcr=$(grep -E "^($ALGORITHM: )*PCRAgg(.*10)*:" hash.txt | tail -1 \
+		| awk '{print $NF}')
+
+	if [ -z "$pcr" ]; then
+		tst_res $res "failed to find aggregate PCR-10 $msg"
+		tst_res TINFO "hash file:"
+		cat hash.txt >&2
+		return
+	fi
+
+	echo "$pcr"
+}
+
 test1_tpm_bypass_mode()
 {
 	local zero=$(echo $DIGEST | awk '{gsub(/./, "0")}; {print}')
@@ -139,8 +227,10 @@ test1_hw_tpm()
 	local cmd="evmctl ima_boot_aggregate -v"
 	local boot_aggregate
 
-	[ -z "$TPM_VERSION" ] && \
+	if [ -z "$TPM_VERSION" ]; then
 		tst_res TWARN "TPM-bypass failed, trying to test TPM device (unknown TPM version)"
+		MAYBE_TPM2=1
+	fi
 
 	if [ "$MISSING_EVMCTL" = 1 ]; then
 		if [ ! -f "$tpm_bios" ]; then
@@ -174,57 +264,57 @@ test1()
 	[ -z "$TPM_VERSION" ] && test1_tpm_bypass_mode || test1_hw_tpm
 }
 
-# Probably cleaner to programmatically read the PCR values directly
-# from the TPM, but that would require a TPM library. For now, use
-# the PCR values from /sys/devices.
-validate_pcr()
+test2()
 {
-	tst_res TINFO "verify PCR (Process Control Register)"
+	local hash pcr_aggregate out ret
 
-	local dev_pcrs="$1"
-	local pcr hash aggregate_pcr
+	tst_res TINFO "verify PCR values"
 
-	aggregate_pcr="$(evmctl -v ima_measurement $BINARY_MEASUREMENTS 2>&1 | \
-		grep 'HW PCR-10:' | awk '{print $3}')"
-	if [ -z "$aggregate_pcr" ]; then
-		tst_res TFAIL "failed to get PCR-10"
-		return 1
+	if [ "$MAYBE_TPM2" = 1 ]; then
+		tst_res TINFO "TMP version not detected ($ERRMSG_TPM), assume TPM 2"
+		TPM_VERSION=2
 	fi
 
-	while read line; do
-		pcr="$(echo $line | cut -d':' -f1)"
-		if [ "$pcr" = "PCR-10" ]; then
-			hash="$(echo $line | cut -d':' -f2 | awk '{ gsub (" ", "", $0); print tolower($0) }')"
-			[ "$hash" = "$aggregate_pcr" ]
-			return $?
-		fi
-	done < $dev_pcrs
-	return 1
-}
+	if [ -z "$TPM_VERSION" ]; then
+		tst_brk TCONF "TMP version not detected ($ERRMSG_TPM)"
+	fi
 
-test2()
-{
-	tst_res TINFO "verify PCR values"
-	tst_check_cmds evmctl || return
+	if [ "$ALGORITHM" = "sha1" -a "$MISSING_EVMCTL" = 1 ]; then
+		tst_check_cmds evmctl || return 1
+	fi
 
-	tst_res TINFO "evmctl version: $(evmctl --version)"
+	out=$(read_pcr_tpm$TPM_VERSION)
+	ret=$?
 
-	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"
+	if [ $ret -ne 0 ]; then
+		case "$ret" in
+			1) tst_res TFAIL "$out";;
+			32) tst_res TCONF "$out";;
+			*) tst_brk TBROK "unsupported return type '$1'";;
+		esac
+		return
 	fi
 
-	if [ -f "$pcrs_path" ]; then
-		validate_pcr $pcrs_path
-		if [ $? -eq 0 ]; then
-			tst_res TPASS "aggregate PCR value matches real PCR value"
-		else
-			tst_res TFAIL "aggregate PCR value does not match real PCR value"
-		fi
+	hash=$(echo "$out" | grep "^PCR-10" | cut -d' ' -f2)
+
+	if [ -z "$out" ]; then
+		tst_res TFAIL "PCR-10 hash not found"
+		return
+	fi
+
+	tst_res TINFO "real PCR-10: '$hash'"
+
+	get_pcr10_aggregate > tmp.txt
+	pcr_aggregate="$(cat tmp.txt)"
+	if [ -z "$pcr_aggregate" ]; then
+		return
+	fi
+	tst_res TINFO "aggregate PCR-10: '$pcr_aggregate'"
+
+	if [ "$hash" = "$pcr_aggregate" ]; then
+		tst_res TPASS "aggregate PCR value matches real PCR value"
 	else
-		tst_res TCONF "TPM Hardware Support not enabled in kernel or no TPM chip found"
+		tst_res TFAIL "aggregate PCR value does not match real PCR value"
 	fi
 }
 
-- 
2.29.2


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

* [LTP] [PATCH v5 4/4] ima_tpm.sh: Fix calculating PCR aggregate
@ 2020-12-14 22:19   ` Petr Vorel
  0 siblings, 0 replies; 30+ messages in thread
From: Petr Vorel @ 2020-12-14 22:19 UTC (permalink / raw)
  To: ltp

for TPM 2.0 and support more evmctl versions.

Because exporting PCR registers for TPM 2.0 has not been upstreamed [1],
we use user space code, which requires evmctl >= 1.3.1 and tsspcrread.
Using evmctl allows to test for TPM devices which does not export event
log (/sys/kernel/security/tpm0/binary_bios_measurements).

For TPM 1.2 read tpm0 device's pcrs file from sysfs. (tss1pcrread could
be also used, but it's not yet packaged by distros.)

For old kernels which use SHA1/MD5, any evmctl version is required (evmctl
ima_measurement was introduced in very old v0.7), but
* newer sysctl path /sys/class/tpm/tpm0/device/pcrs requires evmctl 1.1
* using ima_policy=tcb requires 1.3.1 due --ignore-violations

We now support output format of ima_measurement command for various
evmctl versions:
* 1.3: "sha256: TPM PCR-10:" (or other algorithm, e.g. "sha1:")
* 1.1-1.2.1: "HW PCR-10:" (the only previously supported format)
* 0.7-1.0: "PCR-10:"

NOTE: we ignore evmctl failure for evmctl < 1.3.1 (missing --ignore-violations,
also evmctl < 1.1 fails with "PCRAgg does not match PCR-10")

As for previous commit fix testing with TPM 2.0 device which does not
export event log (/sys/kernel/security/tpm0/binary_bios_measurements):
not wrongly assuming TPM-bypass when kernel didn't export other TPM
2.0 files we check in get_tpm_version() but bios boot aggregate is
correct (i.e. not 0x00s). In that case evmctl ima_boot_aggregate can get
boot aggregate even without TPM event log.

[1] https://patchwork.kernel.org/patch/11759729/

Co-developed-by: Mimi Zohar <zohar@linux.ibm.com>
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
Changes v4->v5:
* improved TPM 2.0 detection (e.g. check for /dev/tpmrm0 and /dev/tpm0)
* test2: if evmctl ima_measurement fails, run again with --ignore-violations
* test2: assume TPM 2, if not detected
* print TPM kernel config
* cleanup

 .../security/integrity/ima/tests/ima_setup.sh |  14 +-
 .../security/integrity/ima/tests/ima_tpm.sh   | 176 +++++++++++++-----
 2 files changed, 144 insertions(+), 46 deletions(-)

diff --git a/testcases/kernel/security/integrity/ima/tests/ima_setup.sh b/testcases/kernel/security/integrity/ima/tests/ima_setup.sh
index dbf8a1db4..59a7ffeac 100644
--- a/testcases/kernel/security/integrity/ima/tests/ima_setup.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_setup.sh
@@ -93,7 +93,7 @@ require_ima_policy_content()
 	fi
 }
 
-require_ima_policy_cmdline()
+check_ima_policy_cmdline()
 {
 	local policy="$1"
 	local i
@@ -101,10 +101,18 @@ require_ima_policy_cmdline()
 	grep -q "ima_$policy" /proc/cmdline && return
 	for i in $(cat /proc/cmdline); do
 		if echo "$i" | grep -q '^ima_policy='; then
-			echo "$i" | grep -q -e "|[ ]*$policy" -e "$policy[ ]*|" -e "=$policy" && return
+			echo "$i" | grep -q -e "|[ ]*$policy" -e "$policy[ ]*|" -e "=$policy" && return 0
 		fi
 	done
-	tst_brk TCONF "IMA measurement tests require builtin IMA $policy policy (e.g. ima_policy=$policy kernel parameter)"
+	return 1
+}
+
+require_ima_policy_cmdline()
+{
+	local policy="$1"
+
+	check_ima_policy_cmdline $policy || \
+		tst_brk TCONF "IMA measurement tests require builtin IMA $policy policy (e.g. ima_policy=$policy kernel parameter)"
 }
 
 mount_helper()
diff --git a/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh b/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
index 195fcb16c..233fdeed8 100755
--- a/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
@@ -7,13 +7,14 @@
 # Verify the boot and PCR aggregates.
 
 TST_CNT=2
-TST_NEEDS_CMDS="awk cut"
+TST_NEEDS_CMDS="awk cut tail"
 TST_SETUP="setup"
 
 . ima_setup.sh
 
 EVMCTL_REQUIRED='1.3.1'
 ERRMSG_EVMCTL="=> install evmctl >= $EVMCTL_REQUIRED"
+ERRMSG_TPM="TPM hardware support not enabled in kernel or no TPM chip found"
 
 setup()
 {
@@ -31,7 +32,7 @@ setup()
 
 	TPM_VERSION="$(get_tpm_version)"
 	if [ -z "$TPM_VERSION" ]; then
-		tst_res TINFO "TPM hardware support not enabled in kernel or no TPM chip found, testing TPM-bypass"
+		tst_res TINFO "$ERRMSG_TPM, testing TPM-bypass"
 	else
 		tst_res TINFO "TMP major version: $TPM_VERSION"
 	fi
@@ -121,6 +122,93 @@ get_tpm_version()
 	fi
 }
 
+read_pcr_tpm1()
+{
+	local pcrs_path="/sys/class/tpm/tpm0/device/pcrs"
+	local evmctl_required="1.1"
+	local hash pcr
+
+	if [ ! -f "$pcrs_path" ]; then
+		pcrs_path="/sys/class/misc/tpm0/device/pcrs"
+	elif ! check_evmctl $evmctl_required; then
+		echo "evmctl >= $evmctl_required required"
+		return 32
+	fi
+
+	if [ ! -f "$pcrs_path" ]; then
+		echo "missing PCR file $pcrs_path ($ERRMSG_TPM)"
+		return 32
+	fi
+
+	while read line; do
+		pcr="$(echo $line | cut -d':' -f1)"
+		hash="$(echo $line | cut -d':' -f2 | awk '{ gsub (" ", "", $0); print tolower($0) }')"
+		echo "$pcr: $hash"
+	done < $pcrs_path
+
+	return 0
+}
+
+# NOTE: TPM 1.2 would require to use tss1pcrread which is not fully adopted
+# by distros yet.
+read_pcr_tpm2()
+{
+	local pcrmax=23
+	local pcrread="tsspcrread -halg $ALGORITHM"
+	local i pcr
+
+	tst_check_cmds tsspcrread || return 1
+
+	for i in $(seq 0 $pcrmax); do
+		pcr=$($pcrread -ha "$i" -ns)
+		if [ $? -ne 0 ]; then
+			echo "tsspcrread failed: $pcr"
+			return 1
+		fi
+		printf "PCR-%02d: %s\n" $i "$pcr"
+	done
+
+	return 0
+}
+
+get_pcr10_aggregate()
+{
+	local cmd="evmctl -vv ima_measurement $BINARY_MEASUREMENTS"
+	local msg="$ERRMSG_EVMCTL"
+	local res=TCONF
+	local pcr ret
+
+	if [ -z "$MISSING_EVMCTL" ]; then
+		msg=
+		res=TFAIL
+	fi
+
+	$cmd > hash.txt 2>&1
+	ret=$?
+	if [ $ret -ne 0 -a -z "$MISSING_EVMCTL" ]; then
+		tst_res TFAIL "evmctl failed, trying with --ignore-violations"
+		cmd="$cmd --ignore-violations"
+		$cmd > hash.txt 2>&1
+		ret=$?
+	elif [ $ret -ne 0 -a "$MISSING_EVMCTL" = 1 ]; then
+		tst_brk TFAIL "evmctl failed $msg"
+	fi
+
+	[ $ret -ne 0 ] && tst_res TWARN "evmctl failed, trying to continue $msg"
+
+	pcr=$(grep -E "^($ALGORITHM: )*PCRAgg(.*10)*:" hash.txt | tail -1 \
+		| awk '{print $NF}')
+
+	if [ -z "$pcr" ]; then
+		tst_res $res "failed to find aggregate PCR-10 $msg"
+		tst_res TINFO "hash file:"
+		cat hash.txt >&2
+		return
+	fi
+
+	echo "$pcr"
+}
+
 test1_tpm_bypass_mode()
 {
 	local zero=$(echo $DIGEST | awk '{gsub(/./, "0")}; {print}')
@@ -139,8 +227,10 @@ test1_hw_tpm()
 	local cmd="evmctl ima_boot_aggregate -v"
 	local boot_aggregate
 
-	[ -z "$TPM_VERSION" ] && \
+	if [ -z "$TPM_VERSION" ]; then
 		tst_res TWARN "TPM-bypass failed, trying to test TPM device (unknown TPM version)"
+		MAYBE_TPM2=1
+	fi
 
 	if [ "$MISSING_EVMCTL" = 1 ]; then
 		if [ ! -f "$tpm_bios" ]; then
@@ -174,57 +264,57 @@ test1()
 	[ -z "$TPM_VERSION" ] && test1_tpm_bypass_mode || test1_hw_tpm
 }
 
-# Probably cleaner to programmatically read the PCR values directly
-# from the TPM, but that would require a TPM library. For now, use
-# the PCR values from /sys/devices.
-validate_pcr()
+test2()
 {
-	tst_res TINFO "verify PCR (Process Control Register)"
+	local hash pcr_aggregate out ret
 
-	local dev_pcrs="$1"
-	local pcr hash aggregate_pcr
+	tst_res TINFO "verify PCR values"
 
-	aggregate_pcr="$(evmctl -v ima_measurement $BINARY_MEASUREMENTS 2>&1 | \
-		grep 'HW PCR-10:' | awk '{print $3}')"
-	if [ -z "$aggregate_pcr" ]; then
-		tst_res TFAIL "failed to get PCR-10"
-		return 1
+	if [ "$MAYBE_TPM2" = 1 ]; then
+		tst_res TINFO "TMP version not detected ($ERRMSG_TPM), assume TPM 2"
+		TPM_VERSION=2
 	fi
 
-	while read line; do
-		pcr="$(echo $line | cut -d':' -f1)"
-		if [ "$pcr" = "PCR-10" ]; then
-			hash="$(echo $line | cut -d':' -f2 | awk '{ gsub (" ", "", $0); print tolower($0) }')"
-			[ "$hash" = "$aggregate_pcr" ]
-			return $?
-		fi
-	done < $dev_pcrs
-	return 1
-}
+	if [ -z "$TPM_VERSION" ]; then
+		tst_brk TCONF "TMP version not detected ($ERRMSG_TPM)"
+	fi
 
-test2()
-{
-	tst_res TINFO "verify PCR values"
-	tst_check_cmds evmctl || return
+	if [ "$ALGORITHM" = "sha1" -a "$MISSING_EVMCTL" = 1 ]; then
+		tst_check_cmds evmctl || return 1
+	fi
 
-	tst_res TINFO "evmctl version: $(evmctl --version)"
+	out=$(read_pcr_tpm$TPM_VERSION)
+	ret=$?
 
-	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"
+	if [ $ret -ne 0 ]; then
+		case "$ret" in
+			1) tst_res TFAIL "$out";;
+			32) tst_res TCONF "$out";;
+			*) tst_brk TBROK "unsupported return type '$1'";;
+		esac
+		return
 	fi
 
-	if [ -f "$pcrs_path" ]; then
-		validate_pcr $pcrs_path
-		if [ $? -eq 0 ]; then
-			tst_res TPASS "aggregate PCR value matches real PCR value"
-		else
-			tst_res TFAIL "aggregate PCR value does not match real PCR value"
-		fi
+	hash=$(echo "$out" | grep "^PCR-10" | cut -d' ' -f2)
+
+	if [ -z "$out" ]; then
+		tst_res TFAIL "PCR-10 hash not found"
+		return
+	fi
+
+	tst_res TINFO "real PCR-10: '$hash'"
+
+	get_pcr10_aggregate > tmp.txt
+	pcr_aggregate="$(cat tmp.txt)"
+	if [ -z "$pcr_aggregate" ]; then
+		return
+	fi
+	tst_res TINFO "aggregate PCR-10: '$pcr_aggregate'"
+
+	if [ "$hash" = "$pcr_aggregate" ]; then
+		tst_res TPASS "aggregate PCR value matches real PCR value"
 	else
-		tst_res TCONF "TPM Hardware Support not enabled in kernel or no TPM chip found"
+		tst_res TFAIL "aggregate PCR value does not match real PCR value"
 	fi
 }
 
-- 
2.29.2


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

* Re: [PATCH v5 0/4] TPM 2.0 fixes in IMA tests
  2020-12-14 22:19 ` [LTP] " Petr Vorel
@ 2020-12-17  5:20   ` Mimi Zohar
  -1 siblings, 0 replies; 30+ messages in thread
From: Mimi Zohar @ 2020-12-17  5:20 UTC (permalink / raw)
  To: Petr Vorel, ltp
  Cc: Mimi Zohar, Lakshmi Ramasubramanian, Tushar Sugandhi, linux-integrity

Hi Petr,

On Mon, 2020-12-14 at 23:19 +0100, Petr Vorel wrote:
> The only problem which bothers me is failure on ima_policy=tcb:
> 
> evmctl ima_measurement /sys/kernel/security/integrity/ima/binary_runtime_measurements -vv
> ...
> sha256: PCRAgg  10: c19866f10132282d4cf20ca45f50078db843f95dc8d1ea8819d0e240cdf3b21c
> sha256: TPM PCR-10: df913daa0437a2365f710f6d93a4f2d37146414425d9aaa60740dc635d187158
> sha256: PCRAgg 10 does not match TPM PCR-10
> Failed to match per TPM bank or SHA1 padded TPM digest(s) (count 1446)
> errno: No such file or directory (2)
> 
> Thus test get failure for the fist run without --ignore-violations
> ...
> ima_tpm 1 TINFO: using command: evmctl ima_boot_aggregate -v
> Using tss2-rc-decode to read PCRs.
> ima_tpm 1 TINFO: IMA boot aggregate: '0756853d9378ff6473966e20610a8d1cb97e4dc613cb87adf5e870c8eb93fd0f'
> ima_tpm 1 TPASS: bios boot aggregate matches IMA boot aggregate
> ima_tpm 2 TINFO: verify PCR values
> ima_tpm 2 TINFO: real PCR-10: '6d8aec6291c0c19efdee50e20899939135be073cd4d6e9063e53386f54f9487d'
> ima_tpm 2 TFAIL: evmctl failed, trying with --ignore-violations
> ima_tpm 2 TINFO: aggregate PCR-10: '6d8aec6291c0c19efdee50e20899939135be073cd4d6e9063e53386f54f9487d'
> ima_tpm 2 TPASS: aggregate PCR value matches real PCR value
> ima_tpm 3 TINFO: AppArmor enabled, this may affect test results
> ima_tpm 3 TINFO: it can be disabled with TST_DISABLE_APPARMOR=1 (requires super/root)
> ima_tpm 3 TINFO: loaded AppArmor profiles: none
> 
> Summary:
> passed   2
> failed   1
> skipped  0
> warnings 0
> 
> IMHO unless this is specific for this particular TPM we should skip test
> if ima_policy=tcb.

No, I don't think so.  Violations are a result of a file being opened
for read and write at the same time.  Opening a file for write, when it
is already open for read, results in a Time of Measure/Time of Use
(ToMToU) violation.  Opening a file for read, when it is already open
for write, results in an open_writer violation.  One of the more common
reasons for these violations are log files.

With the builtin TCB measurement policy enabled on the boot command
line, files are measured from the beginning, before a custom policy is
loaded.  Normally a custom policy is loaded after an LSM policy has
been loaded, allowing IMA policy rules to be defined in terms of LSM
labels.

Verifying the IMA measurement list against the TPM PCRs is an important
test.  Ignoring violations doesn't make sense either.   Perhaps if a
custom policy has not been loaded, emit an informational message and
skip the test without "--ignore-violations".

thanks,

Mimi


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

* [LTP] [PATCH v5 0/4] TPM 2.0 fixes in IMA tests
@ 2020-12-17  5:20   ` Mimi Zohar
  0 siblings, 0 replies; 30+ messages in thread
From: Mimi Zohar @ 2020-12-17  5:20 UTC (permalink / raw)
  To: ltp

Hi Petr,

On Mon, 2020-12-14 at 23:19 +0100, Petr Vorel wrote:
> The only problem which bothers me is failure on ima_policy=tcb:
> 
> evmctl ima_measurement /sys/kernel/security/integrity/ima/binary_runtime_measurements -vv
> ...
> sha256: PCRAgg  10: c19866f10132282d4cf20ca45f50078db843f95dc8d1ea8819d0e240cdf3b21c
> sha256: TPM PCR-10: df913daa0437a2365f710f6d93a4f2d37146414425d9aaa60740dc635d187158
> sha256: PCRAgg 10 does not match TPM PCR-10
> Failed to match per TPM bank or SHA1 padded TPM digest(s) (count 1446)
> errno: No such file or directory (2)
> 
> Thus test get failure for the fist run without --ignore-violations
> ...
> ima_tpm 1 TINFO: using command: evmctl ima_boot_aggregate -v
> Using tss2-rc-decode to read PCRs.
> ima_tpm 1 TINFO: IMA boot aggregate: '0756853d9378ff6473966e20610a8d1cb97e4dc613cb87adf5e870c8eb93fd0f'
> ima_tpm 1 TPASS: bios boot aggregate matches IMA boot aggregate
> ima_tpm 2 TINFO: verify PCR values
> ima_tpm 2 TINFO: real PCR-10: '6d8aec6291c0c19efdee50e20899939135be073cd4d6e9063e53386f54f9487d'
> ima_tpm 2 TFAIL: evmctl failed, trying with --ignore-violations
> ima_tpm 2 TINFO: aggregate PCR-10: '6d8aec6291c0c19efdee50e20899939135be073cd4d6e9063e53386f54f9487d'
> ima_tpm 2 TPASS: aggregate PCR value matches real PCR value
> ima_tpm 3 TINFO: AppArmor enabled, this may affect test results
> ima_tpm 3 TINFO: it can be disabled with TST_DISABLE_APPARMOR=1 (requires super/root)
> ima_tpm 3 TINFO: loaded AppArmor profiles: none
> 
> Summary:
> passed   2
> failed   1
> skipped  0
> warnings 0
> 
> IMHO unless this is specific for this particular TPM we should skip test
> if ima_policy=tcb.

No, I don't think so.  Violations are a result of a file being opened
for read and write at the same time.  Opening a file for write, when it
is already open for read, results in a Time of Measure/Time of Use
(ToMToU) violation.  Opening a file for read, when it is already open
for write, results in an open_writer violation.  One of the more common
reasons for these violations are log files.

With the builtin TCB measurement policy enabled on the boot command
line, files are measured from the beginning, before a custom policy is
loaded.  Normally a custom policy is loaded after an LSM policy has
been loaded, allowing IMA policy rules to be defined in terms of LSM
labels.

Verifying the IMA measurement list against the TPM PCRs is an important
test.  Ignoring violations doesn't make sense either.   Perhaps if a
custom policy has not been loaded, emit an informational message and
skip the test without "--ignore-violations".

thanks,

Mimi


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

* Re: [PATCH v5 0/4] TPM 2.0 fixes in IMA tests
  2020-12-17  5:20   ` [LTP] " Mimi Zohar
@ 2020-12-17  8:33     ` Petr Vorel
  -1 siblings, 0 replies; 30+ messages in thread
From: Petr Vorel @ 2020-12-17  8:33 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: ltp, Mimi Zohar, Lakshmi Ramasubramanian, Tushar Sugandhi,
	linux-integrity

Hi Mimi,

> Hi Petr,

> On Mon, 2020-12-14 at 23:19 +0100, Petr Vorel wrote:
> > The only problem which bothers me is failure on ima_policy=tcb:

> > evmctl ima_measurement /sys/kernel/security/integrity/ima/binary_runtime_measurements -vv
> > ...
> > sha256: PCRAgg  10: c19866f10132282d4cf20ca45f50078db843f95dc8d1ea8819d0e240cdf3b21c
> > sha256: TPM PCR-10: df913daa0437a2365f710f6d93a4f2d37146414425d9aaa60740dc635d187158
> > sha256: PCRAgg 10 does not match TPM PCR-10
> > Failed to match per TPM bank or SHA1 padded TPM digest(s) (count 1446)
> > errno: No such file or directory (2)

> > Thus test get failure for the fist run without --ignore-violations
> > ...
> > ima_tpm 1 TINFO: using command: evmctl ima_boot_aggregate -v
> > Using tss2-rc-decode to read PCRs.
> > ima_tpm 1 TINFO: IMA boot aggregate: '0756853d9378ff6473966e20610a8d1cb97e4dc613cb87adf5e870c8eb93fd0f'
> > ima_tpm 1 TPASS: bios boot aggregate matches IMA boot aggregate
> > ima_tpm 2 TINFO: verify PCR values
> > ima_tpm 2 TINFO: real PCR-10: '6d8aec6291c0c19efdee50e20899939135be073cd4d6e9063e53386f54f9487d'
> > ima_tpm 2 TFAIL: evmctl failed, trying with --ignore-violations
> > ima_tpm 2 TINFO: aggregate PCR-10: '6d8aec6291c0c19efdee50e20899939135be073cd4d6e9063e53386f54f9487d'
> > ima_tpm 2 TPASS: aggregate PCR value matches real PCR value
> > ima_tpm 3 TINFO: AppArmor enabled, this may affect test results
> > ima_tpm 3 TINFO: it can be disabled with TST_DISABLE_APPARMOR=1 (requires super/root)
> > ima_tpm 3 TINFO: loaded AppArmor profiles: none

> > Summary:
> > passed   2
> > failed   1
> > skipped  0
> > warnings 0

> > IMHO unless this is specific for this particular TPM we should skip test
> > if ima_policy=tcb.

> No, I don't think so.  Violations are a result of a file being opened
> for read and write at the same time.  Opening a file for write, when it
> is already open for read, results in a Time of Measure/Time of Use
> (ToMToU) violation.  Opening a file for read, when it is already open
> for write, results in an open_writer violation.  One of the more common
> reasons for these violations are log files.

> With the builtin TCB measurement policy enabled on the boot command
> line, files are measured from the beginning, before a custom policy is
> loaded.  Normally a custom policy is loaded after an LSM policy has
> been loaded, allowing IMA policy rules to be defined in terms of LSM
> labels.

> Verifying the IMA measurement list against the TPM PCRs is an important
> test.  Ignoring violations doesn't make sense either.   Perhaps if a
> custom policy has not been loaded, emit an informational message and
> skip the test without "--ignore-violations".

Thanks for an explanation. Agree, you're right. It's most likely wrong setup
(there were some temporary files in /tmp and even postfix pid file in /var/run/),
I need to properly setup dracut-ima. It'd be then good to document this, but I'd
do it as separate effort.

So, can I merge the patchset with your ack/review-by?

Kind regards,
Petr

> thanks,

> Mimi


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

* [LTP] [PATCH v5 0/4] TPM 2.0 fixes in IMA tests
@ 2020-12-17  8:33     ` Petr Vorel
  0 siblings, 0 replies; 30+ messages in thread
From: Petr Vorel @ 2020-12-17  8:33 UTC (permalink / raw)
  To: ltp

Hi Mimi,

> Hi Petr,

> On Mon, 2020-12-14 at 23:19 +0100, Petr Vorel wrote:
> > The only problem which bothers me is failure on ima_policy=tcb:

> > evmctl ima_measurement /sys/kernel/security/integrity/ima/binary_runtime_measurements -vv
> > ...
> > sha256: PCRAgg  10: c19866f10132282d4cf20ca45f50078db843f95dc8d1ea8819d0e240cdf3b21c
> > sha256: TPM PCR-10: df913daa0437a2365f710f6d93a4f2d37146414425d9aaa60740dc635d187158
> > sha256: PCRAgg 10 does not match TPM PCR-10
> > Failed to match per TPM bank or SHA1 padded TPM digest(s) (count 1446)
> > errno: No such file or directory (2)

> > Thus test get failure for the fist run without --ignore-violations
> > ...
> > ima_tpm 1 TINFO: using command: evmctl ima_boot_aggregate -v
> > Using tss2-rc-decode to read PCRs.
> > ima_tpm 1 TINFO: IMA boot aggregate: '0756853d9378ff6473966e20610a8d1cb97e4dc613cb87adf5e870c8eb93fd0f'
> > ima_tpm 1 TPASS: bios boot aggregate matches IMA boot aggregate
> > ima_tpm 2 TINFO: verify PCR values
> > ima_tpm 2 TINFO: real PCR-10: '6d8aec6291c0c19efdee50e20899939135be073cd4d6e9063e53386f54f9487d'
> > ima_tpm 2 TFAIL: evmctl failed, trying with --ignore-violations
> > ima_tpm 2 TINFO: aggregate PCR-10: '6d8aec6291c0c19efdee50e20899939135be073cd4d6e9063e53386f54f9487d'
> > ima_tpm 2 TPASS: aggregate PCR value matches real PCR value
> > ima_tpm 3 TINFO: AppArmor enabled, this may affect test results
> > ima_tpm 3 TINFO: it can be disabled with TST_DISABLE_APPARMOR=1 (requires super/root)
> > ima_tpm 3 TINFO: loaded AppArmor profiles: none

> > Summary:
> > passed   2
> > failed   1
> > skipped  0
> > warnings 0

> > IMHO unless this is specific for this particular TPM we should skip test
> > if ima_policy=tcb.

> No, I don't think so.  Violations are a result of a file being opened
> for read and write at the same time.  Opening a file for write, when it
> is already open for read, results in a Time of Measure/Time of Use
> (ToMToU) violation.  Opening a file for read, when it is already open
> for write, results in an open_writer violation.  One of the more common
> reasons for these violations are log files.

> With the builtin TCB measurement policy enabled on the boot command
> line, files are measured from the beginning, before a custom policy is
> loaded.  Normally a custom policy is loaded after an LSM policy has
> been loaded, allowing IMA policy rules to be defined in terms of LSM
> labels.

> Verifying the IMA measurement list against the TPM PCRs is an important
> test.  Ignoring violations doesn't make sense either.   Perhaps if a
> custom policy has not been loaded, emit an informational message and
> skip the test without "--ignore-violations".

Thanks for an explanation. Agree, you're right. It's most likely wrong setup
(there were some temporary files in /tmp and even postfix pid file in /var/run/),
I need to properly setup dracut-ima. It'd be then good to document this, but I'd
do it as separate effort.

So, can I merge the patchset with your ack/review-by?

Kind regards,
Petr

> thanks,

> Mimi


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

* Re: [PATCH v5 1/4] IMA: Move get_algorithm_digest(), set_digest_index() to ima_setup.sh
  2020-12-14 22:19   ` [LTP] " Petr Vorel
@ 2020-12-17 16:56     ` Mimi Zohar
  -1 siblings, 0 replies; 30+ messages in thread
From: Mimi Zohar @ 2020-12-17 16:56 UTC (permalink / raw)
  To: Petr Vorel, ltp
  Cc: Mimi Zohar, Lakshmi Ramasubramanian, Tushar Sugandhi, linux-integrity

Hi Petr,

Thank you!

On Mon, 2020-12-14 at 23:19 +0100, Petr Vorel wrote:
> To be reusable by more tests (preparation for next commit).
> 
> Call set_digest_index() inside get_algorithm_digest() if needed
> instead of expecting get_algorithm_digest() caller to call
> set_digest_index() before.

For the existing builtin templates, the algorithm/digest field is
consistent.  With support for per policy rule template formats, there
isn't necessarily a single template format for the entire list.

In the future "set_digest_index" might need to be renamed to
"get_digest_index" and the lookup done for each line.

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

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


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

* [LTP] [PATCH v5 1/4] IMA: Move get_algorithm_digest(), set_digest_index() to ima_setup.sh
@ 2020-12-17 16:56     ` Mimi Zohar
  0 siblings, 0 replies; 30+ messages in thread
From: Mimi Zohar @ 2020-12-17 16:56 UTC (permalink / raw)
  To: ltp

Hi Petr,

Thank you!

On Mon, 2020-12-14 at 23:19 +0100, Petr Vorel wrote:
> To be reusable by more tests (preparation for next commit).
> 
> Call set_digest_index() inside get_algorithm_digest() if needed
> instead of expecting get_algorithm_digest() caller to call
> set_digest_index() before.

For the existing builtin templates, the algorithm/digest field is
consistent.  With support for per policy rule template formats, there
isn't necessarily a single template format for the entire list.

In the future "set_digest_index" might need to be renamed to
"get_digest_index" and the lookup done for each line.

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

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


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

* Re: [PATCH v5 3/4] ima_tpm.sh: Fix calculating boot aggregate
  2020-12-14 22:19   ` [LTP] " Petr Vorel
@ 2020-12-17 18:12     ` Mimi Zohar
  -1 siblings, 0 replies; 30+ messages in thread
From: Mimi Zohar @ 2020-12-17 18:12 UTC (permalink / raw)
  To: Petr Vorel, ltp
  Cc: Mimi Zohar, Lakshmi Ramasubramanian, Tushar Sugandhi, linux-integrity

Hi Petr,

On Mon, 2020-12-14 at 23:19 +0100, Petr Vorel wrote:
> for TPM 2.0 or kernel >= v5.8-rc1:
> 6f1a1d103b48 ima: ("Switch to ima_hash_algo for boot aggregate")
> 
> Test still fails with newer TPM 2.0 on kernel < v5.8-rc1.

The above commit was backported in stable.  Do you know if the failing
systems backported the above patch?   I've recently asked for commit
20c59ce010f8 ("ima: extend boot_aggregate with kernel measurements")
also be backported.

> 
> Test was failing, because it expect SHA1 (we ignore MD5) hash, but for TPM 2.0
> is now used IMA default hash algorithm (by default default SHA256).
> This is similar for entries in IMA measurement list so we can reuse
> already existing code.
> 
> Reading other algorithms than SHA1 or support TPM 2.0 requires evmctl
> >= 1.3.1 (1.3 would also work for test1, but will be required for test2).
> 
> Although recent evmctl is recommended, to support older kernels and TPMs
> which support only SHA1, get boot aggregate with old our legacy
> ima_boot_aggregate.c.

^ the LTP legacy ima_boot_aggregate.c still works, without the evmctl
dependency.

> 
> Also fixed cases:
> * testing with no TPM device:
> * TPM TPM 2.0 devices which does not export event log
> (/sys/kernel/security/tpm0/binary_bios_measurements).

^ firmware which does not export the TPM 2.0 binary event log

> 
> Also fixed test without TPM device (when IMA TPM-bypass is tested)
> as some TPM 2.0 devices does not export event log
> (/sys/kernel/security/tpm0/binary_bios_measurements).

This looks like a duplicate of above.  Maybe just add another bullet
*
detecting IMA TPM-bypass mode

> This does not require evmctl at all.

I assume this comment refers to TPM 2.0 calculating the
"boot_aggregate" based on the existing PCR values, as opposed to TPM
1.2 which first walks the TPM event log, calculating the PCRs.

> 
> Also try best to detect TPM major version (1, 2 or none - assume
> TPM-bypass). This fixes testing with TPM 2.0 device which does not
> export event log (/sys/kernel/security/tpm0/binary_bios_measurements):
> not wrongly assuming TPM-bypass when kernel didn't export other TPM
> 2.0 files we check in get_tpm_version() but bios boot aggregate is
> correct (i.e. not 0x00s). In that case evmctl ima_boot_aggregate can get
> boot aggregate even without TPM event log.
> 
> Co-developed-by: Mimi Zohar <zohar@linux.ibm.com>
> Signed-off-by: Petr Vorel <pvorel@suse.cz>

Thanks, Petr!

Mimi


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

* [LTP] [PATCH v5 3/4] ima_tpm.sh: Fix calculating boot aggregate
@ 2020-12-17 18:12     ` Mimi Zohar
  0 siblings, 0 replies; 30+ messages in thread
From: Mimi Zohar @ 2020-12-17 18:12 UTC (permalink / raw)
  To: ltp

Hi Petr,

On Mon, 2020-12-14 at 23:19 +0100, Petr Vorel wrote:
> for TPM 2.0 or kernel >= v5.8-rc1:
> 6f1a1d103b48 ima: ("Switch to ima_hash_algo for boot aggregate")
> 
> Test still fails with newer TPM 2.0 on kernel < v5.8-rc1.

The above commit was backported in stable.  Do you know if the failing
systems backported the above patch?   I've recently asked for commit
20c59ce010f8 ("ima: extend boot_aggregate with kernel measurements")
also be backported.

> 
> Test was failing, because it expect SHA1 (we ignore MD5) hash, but for TPM 2.0
> is now used IMA default hash algorithm (by default default SHA256).
> This is similar for entries in IMA measurement list so we can reuse
> already existing code.
> 
> Reading other algorithms than SHA1 or support TPM 2.0 requires evmctl
> >= 1.3.1 (1.3 would also work for test1, but will be required for test2).
> 
> Although recent evmctl is recommended, to support older kernels and TPMs
> which support only SHA1, get boot aggregate with old our legacy
> ima_boot_aggregate.c.

^ the LTP legacy ima_boot_aggregate.c still works, without the evmctl
dependency.

> 
> Also fixed cases:
> * testing with no TPM device:
> * TPM TPM 2.0 devices which does not export event log
> (/sys/kernel/security/tpm0/binary_bios_measurements).

^ firmware which does not export the TPM 2.0 binary event log

> 
> Also fixed test without TPM device (when IMA TPM-bypass is tested)
> as some TPM 2.0 devices does not export event log
> (/sys/kernel/security/tpm0/binary_bios_measurements).

This looks like a duplicate of above.  Maybe just add another bullet
*
detecting IMA TPM-bypass mode

> This does not require evmctl at all.

I assume this comment refers to TPM 2.0 calculating the
"boot_aggregate" based on the existing PCR values, as opposed to TPM
1.2 which first walks the TPM event log, calculating the PCRs.

> 
> Also try best to detect TPM major version (1, 2 or none - assume
> TPM-bypass). This fixes testing with TPM 2.0 device which does not
> export event log (/sys/kernel/security/tpm0/binary_bios_measurements):
> not wrongly assuming TPM-bypass when kernel didn't export other TPM
> 2.0 files we check in get_tpm_version() but bios boot aggregate is
> correct (i.e. not 0x00s). In that case evmctl ima_boot_aggregate can get
> boot aggregate even without TPM event log.
> 
> Co-developed-by: Mimi Zohar <zohar@linux.ibm.com>
> Signed-off-by: Petr Vorel <pvorel@suse.cz>

Thanks, Petr!

Mimi


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

* Re: [PATCH v5 4/4] ima_tpm.sh: Fix calculating PCR aggregate
  2020-12-14 22:19   ` [LTP] " Petr Vorel
@ 2020-12-17 19:16     ` Mimi Zohar
  -1 siblings, 0 replies; 30+ messages in thread
From: Mimi Zohar @ 2020-12-17 19:16 UTC (permalink / raw)
  To: Petr Vorel, ltp
  Cc: Mimi Zohar, Lakshmi Ramasubramanian, Tushar Sugandhi, linux-integrity

On Mon, 2020-12-14 at 23:19 +0100, Petr Vorel wrote:
> for TPM 2.0 and support more evmctl versions.
> 
> Because exporting PCR registers for TPM 2.0 has not been upstreamed [1],
> we use user space code, which requires evmctl >= 1.3.1 and tsspcrread.

Yes, really annoying.

> Using evmctl allows to test for TPM devices which does not export event
> log (/sys/kernel/security/tpm0/binary_bios_measurements).

Interesting way of phrasing the lack of a TPM 2.0 event log parser in
ima-evm-utils.   Until someone contributes a TPM 2.0 event log parser,
we're dependent on users verifying the event log against the TPM 2.0
PCR banks some other way (e.g. "tsseventextend -sim -if
/sys/kernel/security/tpm0/binary_bios_measurements -ns").

> 
> For TPM 1.2 read tpm0 device's pcrs file from sysfs. (tss1pcrread could
> be also used, but it's not yet packaged by distros.)
> 
> For old kernels which use SHA1/MD5, any evmctl version is required (evmctl
> ima_measurement was introduced in very old v0.7), but
> * newer sysctl path /sys/class/tpm/tpm0/device/pcrs requires evmctl 1.1
> * using ima_policy=tcb requires 1.3.1 due --ignore-violations
> 
> We now support output format of ima_measurement command for various
> evmctl versions:
> * 1.3: "sha256: TPM PCR-10:" (or other algorithm, e.g. "sha1:")
> * 1.1-1.2.1: "HW PCR-10:" (the only previously supported format)
> * 0.7-1.0: "PCR-10:"
> 
> NOTE: we ignore evmctl failure for evmctl < 1.3.1 (missing --ignore-violations,
> also evmctl < 1.1 fails with "PCRAgg does not match PCR-10")
> 
> As for previous commit fix testing with TPM 2.0 device which does not
> export event log (/sys/kernel/security/tpm0/binary_bios_measurements):
> not wrongly assuming TPM-bypass when kernel didn't export other TPM
> 2.0 files we check in get_tpm_version() but bios boot aggregate is
> correct (i.e. not 0x00s). In that case evmctl ima_boot_aggregate can get
> boot aggregate even without TPM event log.
> 
> [1] https://patchwork.kernel.org/patch/11759729/
> 
> Co-developed-by: Mimi Zohar <zohar@linux.ibm.com>
> Signed-off-by: Petr Vorel <pvorel@suse.cz>

Thanks, Petr!

Other than the typo below, it looks good.

Mimi


> +test2()
>  {
> -	tst_res TINFO "verify PCR (Process Control Register)"
> +	local hash pcr_aggregate out ret
>  
> -	local dev_pcrs="$1"
> -	local pcr hash aggregate_pcr
> +	tst_res TINFO "verify PCR values"
>  
> -	aggregate_pcr="$(evmctl -v ima_measurement $BINARY_MEASUREMENTS 2>&1 | \
> -		grep 'HW PCR-10:' | awk '{print $3}')"
> -	if [ -z "$aggregate_pcr" ]; then
> -		tst_res TFAIL "failed to get PCR-10"
> -		return 1
> +	if [ "$MAYBE_TPM2" = 1 ]; then
> +		tst_res TINFO "TMP version not detected ($ERRMSG_TPM), assume TPM 2"


^ TPM  above and below

> +		TPM_VERSION=2
>  	fi
>  
> -	while read line; do
> -		pcr="$(echo $line | cut -d':' -f1)"
> -		if [ "$pcr" = "PCR-10" ]; then
> -			hash="$(echo $line | cut -d':' -f2 | awk '{ gsub (" ", "", $0); print tolower($0) }')"
> -			[ "$hash" = "$aggregate_pcr" ]
> -			return $?
> -		fi
> -	done < $dev_pcrs
> -	return 1
> -}
> +	if [ -z "$TPM_VERSION" ]; then
> +		tst_brk TCONF "TMP version not detected ($ERRMSG_TPM)"

 and here


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

* [LTP] [PATCH v5 4/4] ima_tpm.sh: Fix calculating PCR aggregate
@ 2020-12-17 19:16     ` Mimi Zohar
  0 siblings, 0 replies; 30+ messages in thread
From: Mimi Zohar @ 2020-12-17 19:16 UTC (permalink / raw)
  To: ltp

On Mon, 2020-12-14 at 23:19 +0100, Petr Vorel wrote:
> for TPM 2.0 and support more evmctl versions.
> 
> Because exporting PCR registers for TPM 2.0 has not been upstreamed [1],
> we use user space code, which requires evmctl >= 1.3.1 and tsspcrread.

Yes, really annoying.

> Using evmctl allows to test for TPM devices which does not export event
> log (/sys/kernel/security/tpm0/binary_bios_measurements).

Interesting way of phrasing the lack of a TPM 2.0 event log parser in
ima-evm-utils.   Until someone contributes a TPM 2.0 event log parser,
we're dependent on users verifying the event log against the TPM 2.0
PCR banks some other way (e.g. "tsseventextend -sim -if
/sys/kernel/security/tpm0/binary_bios_measurements -ns").

> 
> For TPM 1.2 read tpm0 device's pcrs file from sysfs. (tss1pcrread could
> be also used, but it's not yet packaged by distros.)
> 
> For old kernels which use SHA1/MD5, any evmctl version is required (evmctl
> ima_measurement was introduced in very old v0.7), but
> * newer sysctl path /sys/class/tpm/tpm0/device/pcrs requires evmctl 1.1
> * using ima_policy=tcb requires 1.3.1 due --ignore-violations
> 
> We now support output format of ima_measurement command for various
> evmctl versions:
> * 1.3: "sha256: TPM PCR-10:" (or other algorithm, e.g. "sha1:")
> * 1.1-1.2.1: "HW PCR-10:" (the only previously supported format)
> * 0.7-1.0: "PCR-10:"
> 
> NOTE: we ignore evmctl failure for evmctl < 1.3.1 (missing --ignore-violations,
> also evmctl < 1.1 fails with "PCRAgg does not match PCR-10")
> 
> As for previous commit fix testing with TPM 2.0 device which does not
> export event log (/sys/kernel/security/tpm0/binary_bios_measurements):
> not wrongly assuming TPM-bypass when kernel didn't export other TPM
> 2.0 files we check in get_tpm_version() but bios boot aggregate is
> correct (i.e. not 0x00s). In that case evmctl ima_boot_aggregate can get
> boot aggregate even without TPM event log.
> 
> [1] https://patchwork.kernel.org/patch/11759729/
> 
> Co-developed-by: Mimi Zohar <zohar@linux.ibm.com>
> Signed-off-by: Petr Vorel <pvorel@suse.cz>

Thanks, Petr!

Other than the typo below, it looks good.

Mimi


> +test2()
>  {
> -	tst_res TINFO "verify PCR (Process Control Register)"
> +	local hash pcr_aggregate out ret
>  
> -	local dev_pcrs="$1"
> -	local pcr hash aggregate_pcr
> +	tst_res TINFO "verify PCR values"
>  
> -	aggregate_pcr="$(evmctl -v ima_measurement $BINARY_MEASUREMENTS 2>&1 | \
> -		grep 'HW PCR-10:' | awk '{print $3}')"
> -	if [ -z "$aggregate_pcr" ]; then
> -		tst_res TFAIL "failed to get PCR-10"
> -		return 1
> +	if [ "$MAYBE_TPM2" = 1 ]; then
> +		tst_res TINFO "TMP version not detected ($ERRMSG_TPM), assume TPM 2"


^ TPM  above and below

> +		TPM_VERSION=2
>  	fi
>  
> -	while read line; do
> -		pcr="$(echo $line | cut -d':' -f1)"
> -		if [ "$pcr" = "PCR-10" ]; then
> -			hash="$(echo $line | cut -d':' -f2 | awk '{ gsub (" ", "", $0); print tolower($0) }')"
> -			[ "$hash" = "$aggregate_pcr" ]
> -			return $?
> -		fi
> -	done < $dev_pcrs
> -	return 1
> -}
> +	if [ -z "$TPM_VERSION" ]; then
> +		tst_brk TCONF "TMP version not detected ($ERRMSG_TPM)"

 and here


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

* Re: [PATCH v5 0/4] TPM 2.0 fixes in IMA tests
  2020-12-17  8:33     ` [LTP] " Petr Vorel
@ 2020-12-17 19:23       ` Mimi Zohar
  -1 siblings, 0 replies; 30+ messages in thread
From: Mimi Zohar @ 2020-12-17 19:23 UTC (permalink / raw)
  To: Petr Vorel
  Cc: ltp, Mimi Zohar, Lakshmi Ramasubramanian, Tushar Sugandhi,
	linux-integrity

On Thu, 2020-12-17 at 09:33 +0100, Petr Vorel wrote:
> Hi Mimi,
> 
> > Hi Petr,
> 
> > On Mon, 2020-12-14 at 23:19 +0100, Petr Vorel wrote:
> > > The only problem which bothers me is failure on ima_policy=tcb:
> 
> > > evmctl ima_measurement /sys/kernel/security/integrity/ima/binary_runtime_measurements -vv
> > > ...
> > > sha256: PCRAgg  10: c19866f10132282d4cf20ca45f50078db843f95dc8d1ea8819d0e240cdf3b21c
> > > sha256: TPM PCR-10: df913daa0437a2365f710f6d93a4f2d37146414425d9aaa60740dc635d187158
> > > sha256: PCRAgg 10 does not match TPM PCR-10
> > > Failed to match per TPM bank or SHA1 padded TPM digest(s) (count 1446)
> > > errno: No such file or directory (2)
> 
> > > Thus test get failure for the fist run without --ignore-violations
> > > ...
> > > ima_tpm 1 TINFO: using command: evmctl ima_boot_aggregate -v
> > > Using tss2-rc-decode to read PCRs.
> > > ima_tpm 1 TINFO: IMA boot aggregate: '0756853d9378ff6473966e20610a8d1cb97e4dc613cb87adf5e870c8eb93fd0f'
> > > ima_tpm 1 TPASS: bios boot aggregate matches IMA boot aggregate
> > > ima_tpm 2 TINFO: verify PCR values
> > > ima_tpm 2 TINFO: real PCR-10: '6d8aec6291c0c19efdee50e20899939135be073cd4d6e9063e53386f54f9487d'
> > > ima_tpm 2 TFAIL: evmctl failed, trying with --ignore-violations
> > > ima_tpm 2 TINFO: aggregate PCR-10: '6d8aec6291c0c19efdee50e20899939135be073cd4d6e9063e53386f54f9487d'
> > > ima_tpm 2 TPASS: aggregate PCR value matches real PCR value
> > > ima_tpm 3 TINFO: AppArmor enabled, this may affect test results
> > > ima_tpm 3 TINFO: it can be disabled with TST_DISABLE_APPARMOR=1 (requires super/root)
> > > ima_tpm 3 TINFO: loaded AppArmor profiles: none
> 
> > > Summary:
> > > passed   2
> > > failed   1
> > > skipped  0
> > > warnings 0
> 
> > > IMHO unless this is specific for this particular TPM we should skip test
> > > if ima_policy=tcb.
> 
> > No, I don't think so.  Violations are a result of a file being opened
> > for read and write at the same time.  Opening a file for write, when it
> > is already open for read, results in a Time of Measure/Time of Use
> > (ToMToU) violation.  Opening a file for read, when it is already open
> > for write, results in an open_writer violation.  One of the more common
> > reasons for these violations are log files.
> 
> > With the builtin TCB measurement policy enabled on the boot command
> > line, files are measured from the beginning, before a custom policy is
> > loaded.  Normally a custom policy is loaded after an LSM policy has
> > been loaded, allowing IMA policy rules to be defined in terms of LSM
> > labels.
> 
> > Verifying the IMA measurement list against the TPM PCRs is an important
> > test.  Ignoring violations doesn't make sense either.   Perhaps if a
> > custom policy has not been loaded, emit an informational message and
> > skip the test without "--ignore-violations".
> 
> Thanks for an explanation. Agree, you're right. It's most likely wrong setup
> (there were some temporary files in /tmp and even postfix pid file in /var/run/),
> I need to properly setup dracut-ima. It'd be then good to document this, but I'd
> do it as separate effort.
> 
> So, can I merge the patchset with your ack/review-by?

Yes, I just finished reviewing the patches.   Other that clarifying the
patch descriptions and fixing the one typo  ("tmp" -> "tpm"), it looks
really.

thanks! 

Mimi


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

* [LTP] [PATCH v5 0/4] TPM 2.0 fixes in IMA tests
@ 2020-12-17 19:23       ` Mimi Zohar
  0 siblings, 0 replies; 30+ messages in thread
From: Mimi Zohar @ 2020-12-17 19:23 UTC (permalink / raw)
  To: ltp

On Thu, 2020-12-17 at 09:33 +0100, Petr Vorel wrote:
> Hi Mimi,
> 
> > Hi Petr,
> 
> > On Mon, 2020-12-14 at 23:19 +0100, Petr Vorel wrote:
> > > The only problem which bothers me is failure on ima_policy=tcb:
> 
> > > evmctl ima_measurement /sys/kernel/security/integrity/ima/binary_runtime_measurements -vv
> > > ...
> > > sha256: PCRAgg  10: c19866f10132282d4cf20ca45f50078db843f95dc8d1ea8819d0e240cdf3b21c
> > > sha256: TPM PCR-10: df913daa0437a2365f710f6d93a4f2d37146414425d9aaa60740dc635d187158
> > > sha256: PCRAgg 10 does not match TPM PCR-10
> > > Failed to match per TPM bank or SHA1 padded TPM digest(s) (count 1446)
> > > errno: No such file or directory (2)
> 
> > > Thus test get failure for the fist run without --ignore-violations
> > > ...
> > > ima_tpm 1 TINFO: using command: evmctl ima_boot_aggregate -v
> > > Using tss2-rc-decode to read PCRs.
> > > ima_tpm 1 TINFO: IMA boot aggregate: '0756853d9378ff6473966e20610a8d1cb97e4dc613cb87adf5e870c8eb93fd0f'
> > > ima_tpm 1 TPASS: bios boot aggregate matches IMA boot aggregate
> > > ima_tpm 2 TINFO: verify PCR values
> > > ima_tpm 2 TINFO: real PCR-10: '6d8aec6291c0c19efdee50e20899939135be073cd4d6e9063e53386f54f9487d'
> > > ima_tpm 2 TFAIL: evmctl failed, trying with --ignore-violations
> > > ima_tpm 2 TINFO: aggregate PCR-10: '6d8aec6291c0c19efdee50e20899939135be073cd4d6e9063e53386f54f9487d'
> > > ima_tpm 2 TPASS: aggregate PCR value matches real PCR value
> > > ima_tpm 3 TINFO: AppArmor enabled, this may affect test results
> > > ima_tpm 3 TINFO: it can be disabled with TST_DISABLE_APPARMOR=1 (requires super/root)
> > > ima_tpm 3 TINFO: loaded AppArmor profiles: none
> 
> > > Summary:
> > > passed   2
> > > failed   1
> > > skipped  0
> > > warnings 0
> 
> > > IMHO unless this is specific for this particular TPM we should skip test
> > > if ima_policy=tcb.
> 
> > No, I don't think so.  Violations are a result of a file being opened
> > for read and write at the same time.  Opening a file for write, when it
> > is already open for read, results in a Time of Measure/Time of Use
> > (ToMToU) violation.  Opening a file for read, when it is already open
> > for write, results in an open_writer violation.  One of the more common
> > reasons for these violations are log files.
> 
> > With the builtin TCB measurement policy enabled on the boot command
> > line, files are measured from the beginning, before a custom policy is
> > loaded.  Normally a custom policy is loaded after an LSM policy has
> > been loaded, allowing IMA policy rules to be defined in terms of LSM
> > labels.
> 
> > Verifying the IMA measurement list against the TPM PCRs is an important
> > test.  Ignoring violations doesn't make sense either.   Perhaps if a
> > custom policy has not been loaded, emit an informational message and
> > skip the test without "--ignore-violations".
> 
> Thanks for an explanation. Agree, you're right. It's most likely wrong setup
> (there were some temporary files in /tmp and even postfix pid file in /var/run/),
> I need to properly setup dracut-ima. It'd be then good to document this, but I'd
> do it as separate effort.
> 
> So, can I merge the patchset with your ack/review-by?

Yes, I just finished reviewing the patches.   Other that clarifying the
patch descriptions and fixing the one typo  ("tmp" -> "tpm"), it looks
really.

thanks! 

Mimi


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

* Re: [PATCH v5 3/4] ima_tpm.sh: Fix calculating boot aggregate
  2020-12-17 18:12     ` [LTP] " Mimi Zohar
@ 2020-12-17 19:36       ` Petr Vorel
  -1 siblings, 0 replies; 30+ messages in thread
From: Petr Vorel @ 2020-12-17 19:36 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: ltp, Mimi Zohar, Lakshmi Ramasubramanian, Tushar Sugandhi,
	linux-integrity

Hi Mimi,

TL;DR: thank you for comments, I update commit message.
Details below.

> Hi Petr,

> On Mon, 2020-12-14 at 23:19 +0100, Petr Vorel wrote:
> > for TPM 2.0 or kernel >= v5.8-rc1:
> > 6f1a1d103b48 ima: ("Switch to ima_hash_algo for boot aggregate")

> > Test still fails with newer TPM 2.0 on kernel < v5.8-rc1.

> The above commit was backported in stable.  Do you know if the failing
> systems backported the above patch?
SLES kernel got it.

> I've recently asked for commit
> 20c59ce010f8 ("ima: extend boot_aggregate with kernel measurements")
> also be backported.
I see, it got backported to v5.4 (LTS).

FYI shell API doesn't have yet support for hint for commits to backport
(my TODO: #700 [1]). Once it's implemented, I'll add 20c59ce010f8 as this tag.

[1] https://github.com/linux-test-project/ltp/issues/700

> > Test was failing, because it expect SHA1 (we ignore MD5) hash, but for TPM 2.0
> > is now used IMA default hash algorithm (by default default SHA256).
> > This is similar for entries in IMA measurement list so we can reuse
> > already existing code.

> > Reading other algorithms than SHA1 or support TPM 2.0 requires evmctl
> > >= 1.3.1 (1.3 would also work for test1, but will be required for test2).

> > Although recent evmctl is recommended, to support older kernels and TPMs
> > which support only SHA1, get boot aggregate with old our legacy
> > ima_boot_aggregate.c.

> ^ the LTP legacy ima_boot_aggregate.c still works, without the evmctl
> dependency.
Yes, I meant in my description this LTP legacy ima_boot_aggregate.c.
Test does not require evmctl for SHA1 hash.

So maybe:
Although recent evmctl is recommended, for older kernels and TPMs
which support only SHA1 is still used the legacy ima_boot_aggregate.c
(no evmctl update required).

> > Also fixed cases:
> > * testing with no TPM device:
> > * TPM TPM 2.0 devices which does not export event log
> > (/sys/kernel/security/tpm0/binary_bios_measurements).

> ^ firmware which does not export the TPM 2.0 binary event log
+1

> > Also fixed test without TPM device (when IMA TPM-bypass is tested)
> > as some TPM 2.0 devices does not export event log
> > (/sys/kernel/security/tpm0/binary_bios_measurements).

> This looks like a duplicate of above.  Maybe just add another bullet
> *
> detecting IMA TPM-bypass mode
+1

> > This does not require evmctl at all.

> I assume this comment refers to TPM 2.0 calculating the
> "boot_aggregate" based on the existing PCR values, as opposed to TPM
> 1.2 which first walks the TPM event log, calculating the PCRs.
No, that's meant for IMA TPM-bypass (test1). Because we only check zero.
Both TPM 1.2 and TPM 2.0 require evmctl for reading PCR-10 (test2).
So just a note for * detecting IMA TPM-bypass mode.

> > Also try best to detect TPM major version (1, 2 or none - assume
> > TPM-bypass). This fixes testing with TPM 2.0 device which does not
> > export event log (/sys/kernel/security/tpm0/binary_bios_measurements):
> > not wrongly assuming TPM-bypass when kernel didn't export other TPM
> > 2.0 files we check in get_tpm_version() but bios boot aggregate is
> > correct (i.e. not 0x00s). In that case evmctl ima_boot_aggregate can get
> > boot aggregate even without TPM event log.

> > Co-developed-by: Mimi Zohar <zohar@linux.ibm.com>
> > Signed-off-by: Petr Vorel <pvorel@suse.cz>

> Thanks, Petr!
Thanks a lot for patient review and info.

Kind regards,
Petr

> Mimi

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

* [LTP] [PATCH v5 3/4] ima_tpm.sh: Fix calculating boot aggregate
@ 2020-12-17 19:36       ` Petr Vorel
  0 siblings, 0 replies; 30+ messages in thread
From: Petr Vorel @ 2020-12-17 19:36 UTC (permalink / raw)
  To: ltp

Hi Mimi,

TL;DR: thank you for comments, I update commit message.
Details below.

> Hi Petr,

> On Mon, 2020-12-14 at 23:19 +0100, Petr Vorel wrote:
> > for TPM 2.0 or kernel >= v5.8-rc1:
> > 6f1a1d103b48 ima: ("Switch to ima_hash_algo for boot aggregate")

> > Test still fails with newer TPM 2.0 on kernel < v5.8-rc1.

> The above commit was backported in stable.  Do you know if the failing
> systems backported the above patch?
SLES kernel got it.

> I've recently asked for commit
> 20c59ce010f8 ("ima: extend boot_aggregate with kernel measurements")
> also be backported.
I see, it got backported to v5.4 (LTS).

FYI shell API doesn't have yet support for hint for commits to backport
(my TODO: #700 [1]). Once it's implemented, I'll add 20c59ce010f8 as this tag.

[1] https://github.com/linux-test-project/ltp/issues/700

> > Test was failing, because it expect SHA1 (we ignore MD5) hash, but for TPM 2.0
> > is now used IMA default hash algorithm (by default default SHA256).
> > This is similar for entries in IMA measurement list so we can reuse
> > already existing code.

> > Reading other algorithms than SHA1 or support TPM 2.0 requires evmctl
> > >= 1.3.1 (1.3 would also work for test1, but will be required for test2).

> > Although recent evmctl is recommended, to support older kernels and TPMs
> > which support only SHA1, get boot aggregate with old our legacy
> > ima_boot_aggregate.c.

> ^ the LTP legacy ima_boot_aggregate.c still works, without the evmctl
> dependency.
Yes, I meant in my description this LTP legacy ima_boot_aggregate.c.
Test does not require evmctl for SHA1 hash.

So maybe:
Although recent evmctl is recommended, for older kernels and TPMs
which support only SHA1 is still used the legacy ima_boot_aggregate.c
(no evmctl update required).

> > Also fixed cases:
> > * testing with no TPM device:
> > * TPM TPM 2.0 devices which does not export event log
> > (/sys/kernel/security/tpm0/binary_bios_measurements).

> ^ firmware which does not export the TPM 2.0 binary event log
+1

> > Also fixed test without TPM device (when IMA TPM-bypass is tested)
> > as some TPM 2.0 devices does not export event log
> > (/sys/kernel/security/tpm0/binary_bios_measurements).

> This looks like a duplicate of above.  Maybe just add another bullet
> *
> detecting IMA TPM-bypass mode
+1

> > This does not require evmctl at all.

> I assume this comment refers to TPM 2.0 calculating the
> "boot_aggregate" based on the existing PCR values, as opposed to TPM
> 1.2 which first walks the TPM event log, calculating the PCRs.
No, that's meant for IMA TPM-bypass (test1). Because we only check zero.
Both TPM 1.2 and TPM 2.0 require evmctl for reading PCR-10 (test2).
So just a note for * detecting IMA TPM-bypass mode.

> > Also try best to detect TPM major version (1, 2 or none - assume
> > TPM-bypass). This fixes testing with TPM 2.0 device which does not
> > export event log (/sys/kernel/security/tpm0/binary_bios_measurements):
> > not wrongly assuming TPM-bypass when kernel didn't export other TPM
> > 2.0 files we check in get_tpm_version() but bios boot aggregate is
> > correct (i.e. not 0x00s). In that case evmctl ima_boot_aggregate can get
> > boot aggregate even without TPM event log.

> > Co-developed-by: Mimi Zohar <zohar@linux.ibm.com>
> > Signed-off-by: Petr Vorel <pvorel@suse.cz>

> Thanks, Petr!
Thanks a lot for patient review and info.

Kind regards,
Petr

> Mimi

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

* Re: [PATCH v5 1/4] IMA: Move get_algorithm_digest(), set_digest_index() to ima_setup.sh
  2020-12-17 16:56     ` [LTP] " Mimi Zohar
@ 2020-12-18 11:27       ` Petr Vorel
  -1 siblings, 0 replies; 30+ messages in thread
From: Petr Vorel @ 2020-12-18 11:27 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: ltp, Mimi Zohar, Lakshmi Ramasubramanian, Tushar Sugandhi,
	linux-integrity

Hi Mimi,

> Hi Petr,

> Thank you!

> On Mon, 2020-12-14 at 23:19 +0100, Petr Vorel wrote:
> > To be reusable by more tests (preparation for next commit).

> > Call set_digest_index() inside get_algorithm_digest() if needed
> > instead of expecting get_algorithm_digest() caller to call
> > set_digest_index() before.

> For the existing builtin templates, the algorithm/digest field is
> consistent.  With support for per policy rule template formats, there
> isn't necessarily a single template format for the entire list.

> In the future "set_digest_index" might need to be renamed to
> "get_digest_index" and the lookup done for each line.
Thanks for info, noted as TODO. Policy example would speedup fixing.


Kind regards,
Petr

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

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


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

* [LTP] [PATCH v5 1/4] IMA: Move get_algorithm_digest(), set_digest_index() to ima_setup.sh
@ 2020-12-18 11:27       ` Petr Vorel
  0 siblings, 0 replies; 30+ messages in thread
From: Petr Vorel @ 2020-12-18 11:27 UTC (permalink / raw)
  To: ltp

Hi Mimi,

> Hi Petr,

> Thank you!

> On Mon, 2020-12-14 at 23:19 +0100, Petr Vorel wrote:
> > To be reusable by more tests (preparation for next commit).

> > Call set_digest_index() inside get_algorithm_digest() if needed
> > instead of expecting get_algorithm_digest() caller to call
> > set_digest_index() before.

> For the existing builtin templates, the algorithm/digest field is
> consistent.  With support for per policy rule template formats, there
> isn't necessarily a single template format for the entire list.

> In the future "set_digest_index" might need to be renamed to
> "get_digest_index" and the lookup done for each line.
Thanks for info, noted as TODO. Policy example would speedup fixing.


Kind regards,
Petr

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

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


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

* Re: [PATCH v5 0/4] TPM 2.0 fixes in IMA tests
  2020-12-17 19:23       ` [LTP] " Mimi Zohar
@ 2020-12-18 11:45         ` Petr Vorel
  -1 siblings, 0 replies; 30+ messages in thread
From: Petr Vorel @ 2020-12-18 11:45 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: ltp, Mimi Zohar, Lakshmi Ramasubramanian, Tushar Sugandhi,
	linux-integrity

Hi Mimi,

> > So, can I merge the patchset with your ack/review-by?

> Yes, I just finished reviewing the patches.   Other that clarifying the
> patch descriptions and fixing the one typo  ("tmp" -> "tpm"), it looks
> really.
Fixed those typos and commit message and finally merged.
Thanks a lot for your several patient reviews and suggestions!

Petr

> thanks! 

> Mimi


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

* [LTP] [PATCH v5 0/4] TPM 2.0 fixes in IMA tests
@ 2020-12-18 11:45         ` Petr Vorel
  0 siblings, 0 replies; 30+ messages in thread
From: Petr Vorel @ 2020-12-18 11:45 UTC (permalink / raw)
  To: ltp

Hi Mimi,

> > So, can I merge the patchset with your ack/review-by?

> Yes, I just finished reviewing the patches.   Other that clarifying the
> patch descriptions and fixing the one typo  ("tmp" -> "tpm"), it looks
> really.
Fixed those typos and commit message and finally merged.
Thanks a lot for your several patient reviews and suggestions!

Petr

> thanks! 

> Mimi


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

* Re: [PATCH v5 1/4] IMA: Move get_algorithm_digest(), set_digest_index() to ima_setup.sh
  2020-12-18 11:27       ` [LTP] " Petr Vorel
@ 2020-12-18 12:10         ` Mimi Zohar
  -1 siblings, 0 replies; 30+ messages in thread
From: Mimi Zohar @ 2020-12-18 12:10 UTC (permalink / raw)
  To: Petr Vorel
  Cc: ltp, Mimi Zohar, Lakshmi Ramasubramanian, Tushar Sugandhi,
	linux-integrity

Hi Petr,

On Fri, 2020-12-18 at 12:27 +0100, Petr Vorel wrote:
> > On Mon, 2020-12-14 at 23:19 +0100, Petr Vorel wrote:
> > > To be reusable by more tests (preparation for next commit).
> 
> > > Call set_digest_index() inside get_algorithm_digest() if needed
> > > instead of expecting get_algorithm_digest() caller to call
> > > set_digest_index() before.
> 
> > For the existing builtin templates, the algorithm/digest field is
> > consistent.  With support for per policy rule template formats, there
> > isn't necessarily a single template format for the entire list.
> 
> > In the future "set_digest_index" might need to be renamed to
> > "get_digest_index" and the lookup done for each line.
> Thanks for info, noted as TODO. Policy example would speedup fixing.

The simplest way of forcing this to happen would be by specifying a
custom format on the boot command line ("ima_template_fmt=") and
measuring buffer data (ima-buf), like the kexec boot command line.

measure func=KEXEC_CMDLINE template=ima-buf

where "template=ima-buf" isn't needed on kernels with commit
dea87d0889dd ("ima: select ima-buf template for buffer measurement") .

thanks,

Mimi


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

* [LTP] [PATCH v5 1/4] IMA: Move get_algorithm_digest(), set_digest_index() to ima_setup.sh
@ 2020-12-18 12:10         ` Mimi Zohar
  0 siblings, 0 replies; 30+ messages in thread
From: Mimi Zohar @ 2020-12-18 12:10 UTC (permalink / raw)
  To: ltp

Hi Petr,

On Fri, 2020-12-18 at 12:27 +0100, Petr Vorel wrote:
> > On Mon, 2020-12-14 at 23:19 +0100, Petr Vorel wrote:
> > > To be reusable by more tests (preparation for next commit).
> 
> > > Call set_digest_index() inside get_algorithm_digest() if needed
> > > instead of expecting get_algorithm_digest() caller to call
> > > set_digest_index() before.
> 
> > For the existing builtin templates, the algorithm/digest field is
> > consistent.  With support for per policy rule template formats, there
> > isn't necessarily a single template format for the entire list.
> 
> > In the future "set_digest_index" might need to be renamed to
> > "get_digest_index" and the lookup done for each line.
> Thanks for info, noted as TODO. Policy example would speedup fixing.

The simplest way of forcing this to happen would be by specifying a
custom format on the boot command line ("ima_template_fmt=") and
measuring buffer data (ima-buf), like the kexec boot command line.

measure func=KEXEC_CMDLINE template=ima-buf

where "template=ima-buf" isn't needed on kernels with commit
dea87d0889dd ("ima: select ima-buf template for buffer measurement") .

thanks,

Mimi


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

end of thread, other threads:[~2020-12-18 12:13 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-14 22:19 [PATCH v5 0/4] TPM 2.0 fixes in IMA tests Petr Vorel
2020-12-14 22:19 ` [LTP] " Petr Vorel
2020-12-14 22:19 ` [PATCH v5 1/4] IMA: Move get_algorithm_digest(), set_digest_index() to ima_setup.sh Petr Vorel
2020-12-14 22:19   ` [LTP] " Petr Vorel
2020-12-17 16:56   ` Mimi Zohar
2020-12-17 16:56     ` [LTP] " Mimi Zohar
2020-12-18 11:27     ` Petr Vorel
2020-12-18 11:27       ` [LTP] " Petr Vorel
2020-12-18 12:10       ` Mimi Zohar
2020-12-18 12:10         ` [LTP] " Mimi Zohar
2020-12-14 22:19 ` [PATCH v5 2/4] IMA: Rewrite ima_boot_aggregate.c to new API Petr Vorel
2020-12-14 22:19   ` [LTP] " Petr Vorel
2020-12-14 22:19 ` [PATCH v5 3/4] ima_tpm.sh: Fix calculating boot aggregate Petr Vorel
2020-12-14 22:19   ` [LTP] " Petr Vorel
2020-12-17 18:12   ` Mimi Zohar
2020-12-17 18:12     ` [LTP] " Mimi Zohar
2020-12-17 19:36     ` Petr Vorel
2020-12-17 19:36       ` [LTP] " Petr Vorel
2020-12-14 22:19 ` [PATCH v5 4/4] ima_tpm.sh: Fix calculating PCR aggregate Petr Vorel
2020-12-14 22:19   ` [LTP] " Petr Vorel
2020-12-17 19:16   ` Mimi Zohar
2020-12-17 19:16     ` [LTP] " Mimi Zohar
2020-12-17  5:20 ` [PATCH v5 0/4] TPM 2.0 fixes in IMA tests Mimi Zohar
2020-12-17  5:20   ` [LTP] " Mimi Zohar
2020-12-17  8:33   ` Petr Vorel
2020-12-17  8:33     ` [LTP] " Petr Vorel
2020-12-17 19:23     ` Mimi Zohar
2020-12-17 19:23       ` [LTP] " Mimi Zohar
2020-12-18 11:45       ` Petr Vorel
2020-12-18 11:45         ` [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.