Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33252 )
Change subject: security/tpm/tss/tcg-2.0: Add multi digits support to tlcl_extend() ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/33252/8/src/security/tpm/tspi/tspi.... File src/security/tpm/tspi/tspi.c:
https://review.coreboot.org/c/coreboot/+/33252/8/src/security/tpm/tspi/tspi.... PS8, Line 206: enum vb2_hash_algorithm digest_algo
We just discovered another subtle but serious problem with this CL. […]
edit: Well... Andrey just told me that I may have still misunderstood some parts of this. It essentially works the way I described, but the data you send with TPM2_Extend still has to be exactly of the size of the hash algorithm used by the TPM, and there's a strong expectation by the spec that you use the same algorithm for both. We were using the wrong type for one of the vboot PCRs which was exposed by this patch and we need to fix that separately.
Still, I really don't think it's a good idea to allow the caller to specify the algorithm used on the TPM for every tpm_extend_pcr() call, because it completely subverts the expectations of how people think PCRs work. When you extend two things into a PCR you think they're both tied together, but if you used different algorithms for them they actually end up being completely separate and orthogonal in the measurement chain. That seems very dangerous to me and I don't think you'd ever really want to use it intentionally that way (if you want to measure things orthogonally, you'd use different PCR numbers instead).
So I would still propose to get rid of the digest_algo parameter here completely and instead either hardcode to SHA256, or if you really need control over it provide a Kconfig to select the algorithm.