Re: [PATCH v7 3/6] tpm: Support boot measurements

[Ilias Apalodimas <ilias.apalodimas@linaro.org>]

Hi Eddie,

The good news, is that this generally seems to be working and is really
close to what I had in mind on code re-usage.  Thanks for the patience!

The bad new now is that I think I found one last (famous last words)
problem

[...]

> +	}
> +
> +	/* Read PCR0 to check if previous firmware extended the PCRs or not. */
> +	rc = tcg2_pcr_read(dev, 0, &digest_list);
> +	if (rc)
> +		return rc;
> +

This is changing how the code used to work and I think the new way of doing
it is wrong.
First of all the check above doesn't check that PCR0 is indeed 0.  It
simply checks we can *read* that PCR.

> +	for (i = 0; i < digest_list.count; ++i) {
> +		len = tpm2_algorithm_to_len(digest_list.digests[i].hash_alg);
> +		for (j = 0; j < len; ++j) {
> +			if (digest_list.digests[i].digest.sha512[j])
> +				break;
> +		}
> +
> +		/* PCR is non-zero; it has been extended, so skip extending. */

I don't think we need this tbh.  The previous bootloader would have either
extended some of the PCRs along with the EventLog construction or he hasn't.
If it did indeed extend the PCRs then PCR0 should be != 0 since it must
contain a measurement of EV_S_CRTM_VERSION. So looking at PCR0 should be
sufficient to trigger replaying the EventLog or not.
If the previous loader managed to mess up somehow,  I don't think it should
be U-Boot's job to fix the mess :)

> +		if (j != len) {
> +			digest_list.count = 0;
> +			break;
> +		}
> +	}
> +
> +	elog->log_position = offsetof(struct tcg_pcr_event, event) + evsz;
> +	rc = tcg2_log_find_end(elog, dev, &digest_list);
> +	if (rc)
> +		return rc;
> +
> +	elog->found = true;
> +	return 0;
> +}
> +

P.S: I did test this using TF-A and re-using the 'forwarded' EventLog.  I
can see all the events replayed correctly apart from the last one, so i'll
keep looking in case something else is missing

Regards
/Ilias