diff mbox series

ima-evm-utils: Improve ima_measurement matching on busy system with >1 banks

Message ID 20210205180739.676410-1-stefanb@linux.ibm.com (mailing list archive)
State New, archived
Headers show
Series ima-evm-utils: Improve ima_measurement matching on busy system with >1 banks | expand

Commit Message

Stefan Berger Feb. 5, 2021, 6:07 p.m. UTC
When a system is very busy with IMA taking measurements into more than
one bank, then we often do not get the PCR 10 values of the sha1 bank
that represents the same log entry as the reading of the PCR value of
the sha256 bank. In other words, the reading of the PCR 10 value from
the sha1 bank may represent the PCR 10 state at the time of the
n-th entry in the log while the reading of the PCR 10 value from the
sha256 bank may represent the state at the time of the (n+1)-th entry.
The result currently is that the PCR measurements do not match and
on a busy system the tool may not easily report a successful match.

This patch fixes this issue by separating the TPM bank comparison for
each one of the banks being looked and using a bit mask for checking
which banks have already been matched. Once the mask has become 0
all PCR banks have been successfully matched.

A run on a busy system may result in the output as follows indicating
PCR bank matches at the n-th entry for the sha1 bank and at a later
entry, possibly n + 1 or n + 2 or so, for the sha256 bank. The
output is interleaved with a match of the sha1 bank against 'padded
matching'.

$ evmctl ima_measurement --ignore-violations /sys/kernel/security/ima/binary_runtime_measurements
Matched bank number 1.
Matched padded bank number 1.
Matched bank number 2.
Matched per TPM bank calculated digest(s).

An idle system may report this here, indicating matches at the n-th
entry.

$ evmctl ima_measurement --ignore-violations /sys/kernel/security/ima/binary_runtime_measurements
Matched bank number 1.
Matched bank number 2.
Matched per TPM bank calculated digest(s).

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
---
 src/evmctl.c | 55 +++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 44 insertions(+), 11 deletions(-)

Comments

Mimi Zohar Feb. 8, 2021, 2:37 a.m. UTC | #1
On Fri, 2021-02-05 at 13:07 -0500, Stefan Berger wrote:
> When a system is very busy with IMA taking measurements into more than
> one bank, then we often do not get the PCR 10 values of the sha1 bank
> that represents the same log entry as the reading of the PCR value of
> the sha256 bank. In other words, the reading of the PCR 10 value from
> the sha1 bank may represent the PCR 10 state at the time of the
> n-th entry in the log while the reading of the PCR 10 value from the
> sha256 bank may represent the state at the time of the (n+1)-th entry.
> The result currently is that the PCR measurements do not match and
> on a busy system the tool may not easily report a successful match.
> 
> This patch fixes this issue by separating the TPM bank comparison for
> each one of the banks being looked and using a bit mask for checking
> which banks have already been matched. Once the mask has become 0
> all PCR banks have been successfully matched.
> 
> A run on a busy system may result in the output as follows indicating
> PCR bank matches at the n-th entry for the sha1 bank and at a later
> entry, possibly n + 1 or n + 2 or so, for the sha256 bank. The
> output is interleaved with a match of the sha1 bank against 'padded
> matching'.
> 
> $ evmctl ima_measurement --ignore-violations /sys/kernel/security/ima/binary_runtime_measurements
> Matched bank number 1.
> Matched padded bank number 1.
> Matched bank number 2.
> Matched per TPM bank calculated digest(s).
> 
> An idle system may report this here, indicating matches at the n-th
> entry.
> 
> $ evmctl ima_measurement --ignore-violations /sys/kernel/security/ima/binary_runtime_measurements
> Matched bank number 1.
> Matched bank number 2.
> Matched per TPM bank calculated digest(s).
> 
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>

Thanks, Stefan.  The patch itself looks good.   However, the "Matched
bank number" debugging info should not be displayed at verbose level 0.
The output, below, is verbose level 1 (-v).  Instead of outputting the
bank matched, including the per bank matched measurement list entry
number would be more interesting.

10 ffdf6aaba2d153397467d9b536f957315cba9f22 ima-sig
sha256:264bc09abacd3d68041f0e1ca767f89cf9543c749684568b7dc747756ea2dc67
/usr/local/bin/evmctl
sha1: PCRAgg  10: 276b70c3b600f94e6f442e8f77e65bbf0586fd8c
sha1: TPM PCR-10: 276b70c3b600f94e6f442e8f77e65bbf0586fd8c
sha1 PCR-10: succeed         <=== include measurement entry number
sha256: PCRAgg  10:
4694ebe62c7df34e212894b175d24e1bfb3c2c16d9d5000852c3cc7492367acb
sha256: TPM PCR-10:
4694ebe62c7df34e212894b175d24e1bfb3c2c16d9d5000852c3cc7492367acb
sha256 PCR-10: succeed     <=== and here
Matched per TPM bank calculated digest(s).

thanks,

Mimi
diff mbox series

Patch

diff --git a/src/evmctl.c b/src/evmctl.c
index 1815f55..67a2407 100644
--- a/src/evmctl.c
+++ b/src/evmctl.c
@@ -1594,10 +1594,15 @@  static struct tpm_bank_info *init_tpm_banks(int *num_banks)
 
 /*
  * Compare the calculated TPM PCR banks against the PCR values read.
+ * The banks_mask parameter allows to select which banks to consider.
+ * A banks_maks of 0x3 would consider banks 1 and 2, 0x2 would only
+ * consider the 2nd bank, ~0 would consider all banks.
+ *
  * On failure to match any TPM bank, fail comparison.
  */
 static int compare_tpm_banks(int num_banks, struct tpm_bank_info *bank,
-			     struct tpm_bank_info *tpm_bank)
+			     struct tpm_bank_info *tpm_bank,
+			     unsigned int banks_mask)
 {
 	int i, j;
 	int ret = 0;
@@ -1605,6 +1610,9 @@  static int compare_tpm_banks(int num_banks, struct tpm_bank_info *bank,
 	for (i = 0; i < num_banks; i++) {
 		if (!bank[i].supported || !tpm_bank[i].supported)
 			continue;
+		if ((banks_mask & (1 << i)) == 0)
+			continue;
+		/* do we need to look at the n-th bank ? */
 		for (j = 0; j < NUM_PCRS; j++) {
 			if (memcmp(bank[i].pcr[j], zero, bank[i].digest_size)
 			    == 0)
@@ -1941,6 +1949,8 @@  static int ima_measurement(const char *file)
 	int num_banks = 0;
 	int tpmbanks = 1;
 	int first_record = 1;
+	unsigned int pseudo_padded_banks_mask, pseudo_banks_mask;
+	int c;
 
 	struct template_entry entry = { .template = 0 };
 	FILE *fp;
@@ -1974,6 +1984,10 @@  static int ima_measurement(const char *file)
 	if (read_tpm_banks(num_banks, tpm_banks) != 0)
 		tpmbanks = 0;
 
+	/* A mask where each bit represents the banks to check against */
+	pseudo_banks_mask = (1 << num_banks) - 1;
+	pseudo_padded_banks_mask = pseudo_banks_mask;
+
 	while (fread(&entry.header, sizeof(entry.header), 1, fp)) {
 		if (entry.header.name_len > TCG_EVENT_NAME_LEN_MAX) {
 			log_err("%d ERROR: event name too long!\n",
@@ -2086,18 +2100,37 @@  static int ima_measurement(const char *file)
 		if (!tpmbanks)
 			continue;
 
-		/* The measurement list might contain too many entries,
-		 * compare the re-calculated TPM PCR values after each
-		 * extend.
-		 */
-		err = compare_tpm_banks(num_banks, pseudo_banks, tpm_banks);
-		if (!err)
+		for (c = 0; c < num_banks; c++) {
+			if ((pseudo_banks_mask & (1 << c)) == 0)
+				continue;
+			/* The measurement list might contain too many entries,
+			 * compare the re-calculated TPM PCR values after each
+			 * extend.
+			 */
+			err = compare_tpm_banks(num_banks, pseudo_banks,
+						tpm_banks, 1 << c);
+			if (!err) {
+				pseudo_banks_mask ^= (1 << c);
+				log_info("Matched bank number %u.\n", c + 1);
+			}
+		}
+		if (pseudo_banks_mask == 0)
 			break;
 
-		/* Compare against original SHA1 zero padded TPM PCR values */
-		err_padded = compare_tpm_banks(num_banks, pseudo_padded_banks,
-					       tpm_banks);
-		if (!err_padded)
+		for (c = 0; c < num_banks; c++) {
+			if ((pseudo_padded_banks_mask & (1 << c)) == 0)
+				continue;
+			/* Compare against original SHA1 zero padded TPM PCR values */
+			err_padded = compare_tpm_banks(num_banks,
+						       pseudo_padded_banks,
+						       tpm_banks,
+						       1 << c);
+			if (!err_padded) {
+				pseudo_padded_banks_mask ^= (1 << c);
+				log_info("Matched padded bank number %u.\n", c + 1);
+			}
+		}
+		if (pseudo_padded_banks_mask == 0)
 			break;
 	}