Hi Eddie,
On Thu, 2 Mar 2023 at 16:17, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
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 :)
I actually misread the patch here. This is indeed only checking PCR0. I'll go check why one event is missing and get back to you.
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