[PATCH] tpmevlog: Ensure the event log matches the actual TPM PCRs

ivanhu ivan.hu at canonical.com
Mon Sep 23 04:08:58 UTC 2024


Hi Jonathan,

Thanks for the patch.

We got a build failure. It seems the Makefile needs to be updated:
/usr/bin/ld: tpm/tpmevlog/fwts-tpmevlog.o: undefined reference to symbol 'g_checksum_free'
/usr/bin/ld: /lib/x86_64-linux-gnu/libglib-2.0.so.0: error adding symbols: DSO missing from the command line.

Additionally, you might want to move those PCR-related functions into src/lib.
This change should avoid the need to modify the Makefile and should work seamlessly.

Cheers,
Ivan


On 9/13/24 22:56, Jonathan McDowell wrote:
> From: Jonathan McDowell <noodles at meta.com>
> 
> Ultimately the TPM event log is used to be able to validate how the
> current TPM PCRs values were arrived at. This is only useful if parsing
> the event log results in the same values as the TPM actually has stored
> in it. Add another minor test to the tpmevlog test that this is the
> case.
> 
> Signed-off-by: Jonathan McDowell <noodles at meta.com>
> ---
> Note: I've added this as an extra minor test so it can be checked
> separately from the main event log sanity checks. It relies on those
> checks having been run, rather than duplicating all the code for log
> parsing. I've only checked the SHA256 hashes, as they're mandated by the
> spec.
> 
>   src/lib/include/fwts_tpm.h  |   3 +
>   src/tpm/tpmevlog/tpmevlog.c | 129 ++++++++++++++++++++++++++++++++++--
>   2 files changed, 127 insertions(+), 5 deletions(-)
> 
> diff --git a/src/lib/include/fwts_tpm.h b/src/lib/include/fwts_tpm.h
> index 4816093d..0f08ca96 100644
> --- a/src/lib/include/fwts_tpm.h
> +++ b/src/lib/include/fwts_tpm.h
> @@ -32,6 +32,9 @@ PRAGMA_PACK_WARN_OFF
>   #define TPM2_SHA384_DIGEST_SIZE  48
>   #define TPM2_SHA512_DIGEST_SIZE  64
>   
> +/* Number of PCRs used for firmware, rather than OS, measurements */
> +#define TPM2_FIRMWARE_PCR_COUNT 8
> +
>   typedef uint16_t TPM2_ALG_ID;
>   #define TPM2_ALG_ERROR               ((TPM2_ALG_ID) 0x0000)
>   #define TPM2_ALG_RSA                 ((TPM2_ALG_ID) 0x0001)
> diff --git a/src/tpm/tpmevlog/tpmevlog.c b/src/tpm/tpmevlog/tpmevlog.c
> index d06638f0..dfcd5f28 100644
> --- a/src/tpm/tpmevlog/tpmevlog.c
> +++ b/src/tpm/tpmevlog/tpmevlog.c
> @@ -19,16 +19,70 @@
>   #include "fwts.h"
>   
>   #include <sys/types.h>
> +#include <ctype.h>
>   #include <dirent.h>
>   #include <errno.h>
>   #include <sys/stat.h>
>   #include <fcntl.h>
> +#include <glib.h>
>   
>   #include "fwts_tpm.h"
>   #include "fwts_uefi.h"
>   
> +#define FWTS_TPM_DEVICE_PATH	"/sys/class/tpm"
>   #define FWTS_TPM_LOG_DIR_PATH	"/sys/kernel/security"
>   
> +struct tpm_pcr_state {
> +	uint8_t pcr[TPM2_FIRMWARE_PCR_COUNT][TPM2_SHA256_DIGEST_SIZE];
> +};
> +
> +static bool tpmevlog_parsed = false;
> +static struct tpm_pcr_state tpmevlog_pcrs = {};
> +
> +static int tpmevlog_read_device_pcrs(fwts_framework *fw, const char *tpm, struct tpm_pcr_state *pcrs)
> +{
> +	char path[PATH_MAX];
> +	char buffer[TPM2_SHA256_DIGEST_SIZE * 2 + 2]; /* Trailing \n + NUL */
> +	int i, j;
> +	int fd;
> +
> +	memset(pcrs, 0, sizeof(*pcrs));
> +	for (i = 0; i < TPM2_FIRMWARE_PCR_COUNT; i++) {
> +		snprintf(path, sizeof(path), FWTS_TPM_DEVICE_PATH "/%s/pcr-sha256/%d",
> +			tpm, i);
> +
> +		if ((fd = open(path, O_RDONLY)) >= 0) {
> +			const ssize_t n = read(fd, buffer, sizeof(buffer) - 1);
> +
> +			if (n <= 0 || buffer[n - 1] != '\n') {
> +				fwts_log_info(fw, "Cannot read TPM PCR%d value.", i);
> +				(void)close(fd);
> +				return FWTS_ERROR;
> +			}
> +
> +			for (j = 0; j < (n - 1); j++) {
> +				char c = toupper(buffer[j]);
> +
> +				if (!isxdigit(c)) {
> +					fwts_log_info(fw, "Cannot parse TPM PCR%d value.", i);
> +					(void)close(fd);
> +					return FWTS_ERROR;
> +				}
> +
> +				if (j & 1) {
> +					pcrs->pcr[i][j >> 1] |= isdigit(c) ? c - '0' : c - 'A' + 10;
> +				} else {
> +					pcrs->pcr[i][j >> 1] = (isdigit(c) ? c - '0' : c - 'A' + 10) << 4;
> +				}
> +			}
> +
> +			(void)close(fd);
> +		}
> +	}
> +
> +	return FWTS_OK;
> +}
> +
>   static int tpmevlog_pcrindex_value_check(fwts_framework *fw, const uint32_t pcr)
>   {
>   	/*
> @@ -200,7 +254,8 @@ static int tpmevlog_v2_check(
>   	fwts_pc_client_pcr_event *pc_event;
>   	fwts_efi_spec_id_event *specid_evcent;
>   	fwts_spec_id_event_alg_sz *alg_sz;
> -	bool separator_seen[8] = { false };
> +	bool separator_seen[TPM2_FIRMWARE_PCR_COUNT] = { false };
> +	bool startuplocality_seen = false;
>   
>   	/* specid_event_check */
>   	if (len < sizeof(fwts_pc_client_pcr_event)) {
> @@ -361,6 +416,25 @@ static int tpmevlog_v2_check(
>   				return FWTS_ERROR;
>   			}
>   
> +			/*
> +			 * Extend the current PCR hash with the new hash from the log
> +			 */
> +			if ((!tpmevlog_parsed) && (alg_id == TPM2_ALG_SHA256) &&
> +			    (pcr_event2->event_type != EV_NO_ACTION) &&
> +			    (pcr_event2->pcr_index < TPM2_FIRMWARE_PCR_COUNT)) {
> +				GChecksum *hasher = g_checksum_new(G_CHECKSUM_SHA256);
> +				gsize hash_len = TPM2_SHA256_DIGEST_SIZE;
> +
> +				g_checksum_update(hasher,
> +						  tpmevlog_pcrs.pcr[pcr_event2->pcr_index],
> +						  TPM2_SHA256_DIGEST_SIZE);
> +				g_checksum_update(hasher, pdata, hash_size);
> +				g_checksum_get_digest(hasher,
> +						  tpmevlog_pcrs.pcr[pcr_event2->pcr_index],
> +						  &hash_len);
> +				g_checksum_free(hasher);
> +			}
> +
>   			pdata += hash_size;
>   			len_remain -= hash_size;
>   		}
> @@ -381,15 +455,33 @@ static int tpmevlog_v2_check(
>   		if (ret != FWTS_OK)
>   			return ret;
>   
> -		if ((pcr_event2->pcr_index < 8) && (pcr_event2->event_type == EV_SEPARATOR))
> +		if ((pcr_event2->pcr_index < TPM2_FIRMWARE_PCR_COUNT) &&
> +		    (pcr_event2->event_type == EV_SEPARATOR))
>   			separator_seen[pcr_event2->pcr_index] = true;
>   
> -		pdata += (event_size + sizeof(event_size));
> -		len_remain -= (event_size + sizeof(event_size));
> +		pdata += sizeof(event_size);
> +		len_remain -= sizeof(event_size);
> +
> +		if (pcr_event2->event_type == EV_NO_ACTION &&
> +		    pcr_event2->pcr_index == 0 &&
> +		    event_size == 17 &&
> +		    memcmp(pdata, "StartupLocality", sizeof("StartupLocality")) == 0) {
> +			if (!startuplocality_seen) {
> +				tpmevlog_pcrs.pcr[0][TPM2_SHA256_DIGEST_SIZE - 1] =
> +					pdata[sizeof("StartupLocality")];
> +				startuplocality_seen = true;
> +			} else {
> +				fwts_failed(fw, LOG_LEVEL_MEDIUM, "EventV2StartupLocalitySeenTwice",
> +					"Event log contained more than one StartupLocality event");
> +				return FWTS_ERROR;
> +			}
> +		}
>   
> +		pdata += event_size;
> +		len_remain -= event_size;
>   	}
>   
> -	for (i = 0; i < 8; i++) {
> +	for (i = 0; i < TPM2_FIRMWARE_PCR_COUNT; i++) {
>   		if (!separator_seen[i]) {
>   			fwts_failed(fw, LOG_LEVEL_MEDIUM, "EventV2SeparatorSeen",
>   				"PCR %d did not have EV_SEPARATOR measured into it at "
> @@ -398,6 +490,7 @@ static int tpmevlog_v2_check(
>   		}
>   	}
>   
> +	tpmevlog_parsed = true;
>   	fwts_passed(fw, "Check TPM crypto agile event log test passed.");
>   	return FWTS_OK;
>   }
> @@ -551,8 +644,34 @@ static int tpmevlog_test1(fwts_framework *fw)
>   	return FWTS_OK;
>   }
>   
> +static int tpmevlog_test2(fwts_framework *fw)
> +{
> +	struct tpm_pcr_state pcrs_actual = {};
> +	if (!tpmevlog_parsed) {
> +		fwts_skipped(fw, "No TPM 2.0 event log has not been parsed, skipping.");
> +		return FWTS_SKIP;
> +	}
> +
> +	if (tpmevlog_read_device_pcrs(fw, "tpm0", &pcrs_actual) != FWTS_OK) {
> +		fwts_skipped(fw, "Could not read PCRs from TPM, skipping.");
> +		return FWTS_SKIP;
> +	}
> +
> +	for (int i = 0; i < TPM2_FIRMWARE_PCR_COUNT; i++) {
> +		if (memcmp(pcrs_actual.pcr[i], tpmevlog_pcrs.pcr[i], TPM2_SHA256_DIGEST_SIZE) != 0) {
> +			fwts_failed(fw, LOG_LEVEL_HIGH, "PCRsMatchEvLog",
> +				"PCR %d differs between actual value and log.", i);
> +			return FWTS_ERROR;
> +		}
> +	}
> +
> +	fwts_passed(fw, "Check TPM event log machines actual TPM PCRs.");
> +	return FWTS_OK;
> +}
> +
>   static fwts_framework_minor_test tpmevlog_tests[] = {
>   	{ tpmevlog_test1, "Sanity check TPM event log." },
> +	{ tpmevlog_test2, "Check TPM event log matches TPM PCRs." },
>   	{ NULL, NULL }
>   };
>   



More information about the fwts-devel mailing list