[PATCH 1/3] ima-evm-utils: similarly add sanity check for file parameter of TPM 1.2 PCRs

[PATCH 1/3] ima-evm-utils: similarly add sanity check for file parameter of TPM 1.2 PCRs

[Mimi Zohar]

Parameter expects to be a copy of /sys/class/tpm/tpm0/device/pcrs (i.e.
regular file, not a directory, block or character device, socket, ...)

Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
 src/evmctl.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/src/evmctl.c b/src/evmctl.c
index 0f1c5a023516..06a2ffb879d9 100644
--- a/src/evmctl.c
+++ b/src/evmctl.c
@@ -1379,14 +1379,26 @@ static char *misc_pcrs = "/sys/class/misc/tpm0/device/pcrs";
 /* Read all of the TPM 1.2 PCRs */
 static int tpm_pcr_read(struct tpm_bank_info *tpm_banks, int len)
 {
+	struct stat s;
 	FILE *fp = NULL;
 	char *p, pcr_str[8], buf[70]; /* length of the TPM string */
 	int result = -1;
 	int i = 0;
 
 	/* Use the provided TPM 1.2 pcrs file */
-	if (pcrfile)
+	if (pcrfile) {
+		if (stat(pcrfile, &s) == -1) {
+			errno = 0;
+			return 1;
+		}
+
+		if (!S_ISREG(s.st_mode)) {
+			log_info("TPM 1.2 PCR file: not a regular file or link to regular file\n");
+			return 1;
+		}
+
 		fp = fopen(pcrfile, "r");
+	}
 
 	if (!fp)
 		fp = fopen(pcrs, "r");
-- 
2.7.5

[PATCH 2/3] ima-evm-utils: output specific "unknown keyid" file msg based on log level

[Mimi Zohar]

When the IMA measurement list contains file signatures, the file
signatures are verified either by calculating the local file data hash
or based on the file hash contained in the measurement list.  In either
case a list of trusted public keys needs to be provided.

In addition to the list of known/unknown public keys needed to verify
the measurement list being output, the specific files signed by an
unknown public key are output as well.

Output the individual "unknown keyid" file messages based on log level.

Example 1: "ima_measurement" list of known/unknown public keys

Verify the provided IMA measurement list against the provided TPM 1.2
PCRs.
--validate: ignore measurement violations.
--verify: calculate and verify the template digest against the template
data.
--verify-sig: verify the file signature against the file hash stored
in the template data.

$ evmctl ima_measurement /tmp/local_binary_runtime_measurements --pcrs
/tmp/local_pcrs_new --validate --verify --verify-sig
key 1: 14c2d147 /etc/keys/x509_evm.der
key 2: 6e6c1046 (unknown keyid)
key 3: c4e2426e (unknown keyid)
Matched per TPM bank calculated digest(s).

Example 2: verbose mode (-v) includes specific unknown files.

/usr/bin/evmctl: verification failed: unknown keyid 6e6c1046

Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
 src/libimaevm.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/libimaevm.c b/src/libimaevm.c
index 16e07e82b9e3..fa6c27858d0f 100644
--- a/src/libimaevm.c
+++ b/src/libimaevm.c
@@ -512,8 +512,9 @@ static int verify_hash_v2(const char *file, const unsigned char *hash, int size,
 	if (!pkey) {
 		uint32_t keyid = hdr->keyid;
 
-		log_info("%s: verification failed: unknown keyid %x\n",
-			 file, __be32_to_cpup(&keyid));
+		if (imaevm_params.verbose > LOG_INFO)
+			log_info("%s: verification failed: unknown keyid %x\n",
+				 file, __be32_to_cpup(&keyid));
 		return -1;
 	}
 
-- 
2.7.5

[PATCH 3/3] ima_evm_utils: indicate "--verify" template data digest failures

[Mimi Zohar]

Helps to indicate when the template data digest verification fails.
Indicate the problematic record in the measurement list based on
log level and fail verification.

fixes: ff26f9704ec4 ("ima-evm-utils: calculate and verify the template
data digest")

Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
 src/evmctl.c | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/src/evmctl.c b/src/evmctl.c
index 06a2ffb879d9..faddc3c361a0 100644
--- a/src/evmctl.c
+++ b/src/evmctl.c
@@ -1444,14 +1444,21 @@ static int verify = 0;
 static int ima_verify_template_hash(struct template_entry *entry)
 {
 	uint8_t digest[SHA_DIGEST_LENGTH];
+	static int line = 0;
+
+	line++;
 
 	if (!memcmp(zero, entry->header.digest, sizeof(digest)))
 		return 0;
 
 	SHA1(entry->template, entry->template_len, digest);
 
-	if (memcmp(digest, entry->header.digest, sizeof(digest)))
+	if (memcmp(digest, entry->header.digest, sizeof(digest))) {
+		if (imaevm_params.verbose > LOG_INFO)
+			log_info("Failed to verify template data digest(line %d).\n",
+				  line);
 		return 1;
+	}
 
 	return 0;
 }
@@ -1892,6 +1899,7 @@ static int ima_measurement(const char *file)
 
 	struct template_entry entry = { .template = 0 };
 	FILE *fp;
+	int verified_template_digest = 0;
 	int err_padded = -1;
 	int err = -1;
 
@@ -2020,8 +2028,12 @@ static int ima_measurement(const char *file)
 		extend_tpm_banks(&entry, num_banks, pseudo_banks,
 				 pseudo_padded_banks);
 
-		if (verify)
-			ima_verify_template_hash(&entry);
+		/* Recalculate and verify template data digest */
+		if (verify) {
+			err = ima_verify_template_hash(&entry);
+			if (err)
+				verified_template_digest = 1;
+		}
 
 		if (is_ima_template)
 			ima_show(&entry);
@@ -2058,6 +2070,11 @@ static int ima_measurement(const char *file)
 			log_info("Failed to match per TPM bank or SHA1 padded TPM digest(s).\n");
 	}
 
+	if (verified_template_digest) {
+		log_info("Failed to verify template data digest.\n");
+		err = 1;
+	}
+
 out:
 	fclose(fp);
 	return err;
-- 
2.7.5

Re: [PATCH 1/3] ima-evm-utils: similarly add sanity check for file parameter of TPM 1.2 PCRs

[Petr Vorel]

HI Mimi,

> Parameter expects to be a copy of /sys/class/tpm/tpm0/device/pcrs (i.e.
> regular file, not a directory, block or character device, socket, ...)

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

Kind regards,
Petr

Re: [PATCH 2/3] ima-evm-utils: output specific "unknown keyid" file msg based on log level

[Petr Vorel]

Hi Mimi,

> When the IMA measurement list contains file signatures, the file
> signatures are verified either by calculating the local file data hash
> or based on the file hash contained in the measurement list.  In either
> case a list of trusted public keys needs to be provided.

> In addition to the list of known/unknown public keys needed to verify
> the measurement list being output, the specific files signed by an
> unknown public key are output as well.

> Output the individual "unknown keyid" file messages based on log level.

> Example 1: "ima_measurement" list of known/unknown public keys

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

Kind regards,
Petr

Re: [PATCH 3/3] ima_evm_utils: indicate "--verify" template data digest failures

[Petr Vorel]

Hi Mimi,

> Helps to indicate when the template data digest verification fails.
> Indicate the problematic record in the measurement list based on
> log level and fail verification.

> fixes: ff26f9704ec4 ("ima-evm-utils: calculate and verify the template
> data digest")

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

Kind regards,
Petr

Re: [PATCH 2/3] ima-evm-utils: output specific "unknown keyid" file msg based on log level

[Petr Vorel]

Hi Mimi,

> -		log_info("%s: verification failed: unknown keyid %x\n",
> -			 file, __be32_to_cpup(&keyid));
> +		if (imaevm_params.verbose > LOG_INFO)
> +			log_info("%s: verification failed: unknown keyid %x\n",

BTW, I was thinking to add more macros which would handle if
(imaevm_params.verbose > LOG_INFO), something like:

+#define log_dump_verbose(p, len)               if (imaevm_params.verbose > LOG_INFO) do_dump(p, len, true)
+#define log_verbose(fmt, args...)              if (imaevm_params.verbose > LOG_INFO) log_info(fmt, ##args)

But in the end I didn't post it, because:
1) imaevm_params.verbose is sometimes used for other purpose:
src/evmctl.c:
	if (sigdump || imaevm_params.verbose >= LOG_INFO)
		imaevm_hexdump(sig, len);
...
	if (imaevm_params.verbose > LOG_INFO) {
		log_info("%d ", entry->header.pcr);
		log_dump_n(entry->header.digest, sizeof(entry->header.digest));
		log_info(" %s %s", entry->name, algo);
		log_dump_n(digest, digest_len);
		log_info(" %s", path);
		if (fbuf) {
			log_info(" ");
			log_dump_n(fbuf, fbuf_len);
		}
	}
...
     if (imaevm_params.verbose <= LOG_INFO)
         return;

2) code sometimes compares: imaevm_params.verbose >= LOG_INFO (i.e. >= vs >; is
that intentional?)

Kind regards,
Petr