[SeaBIOS] [PATCH v2 3/6] tpm: Extend tpm20_extend to support extending to multiple PCR banks

[Stefan Berger]

On 07/28/2016 12:54 PM, Kevin O'Connor wrote:
> On Tue, Jul 26, 2016 at 11:19:50AM -0400, Stefan Berger wrote:
>> Extend the tpm20_extend function to support extending a hash to
>> multiple PCR banks. The sha1 hash that's being extended into the
>> sha256 bank for example, will be filled with zero-bytes to the
>> size of a sha256 hash.
> [...]
>> +/*
>> + * Write the TPM2 tpml_digest_values data structure from the given hash.
>> + * Follow the PCR bank configuration of the TPM and write the same hash
>> + * in either truncated or zero-padded form in the areas of all the other
>> + * hashes. For example, write the sha1 hash in the area of the sha256
>> + * hash and fill the remaining bytes with zeros. Or truncate the sha256
>> + * hash when writing it in the area of the sha1 hash.
>> + *
>> + * dest: destination buffer to write into; if NULL, nothing is written
>> + * destlen: length of destination buffer
>> + * pcrindex: the PCR index
>> + * hash: the hash value to write
>> + * hashAlg: the hash alogorithm determining the size of the hash
>> + * count: pointer to a counter for how many entries were writte
>> + *
>> + * Returns the number of bytes needed in the buffer; -1 on fatal error
>> + */
>> +static int
>> +tpm20_write_tpml_dig_values(u8 *dest, size_t destlen, u32 pcrindex,
>> +                            const u8 *hash, u16 hashAlg)
> So, if I understand this correctly, the current code creates a
> "digest" with just a sha1 hash.  However, the hardware has a
> description of what the digest should look like, and this patch takes
> the current digest and produces the digest format desired by the
> hardware.  Patch 6 does the same for the log.

The TPM 2 supports multiple PCR banks with SHA1 , SHA256, SHA384 etc. 
Multiple of these banks can be active at the same time. To avoid abuse 
of these banks during attestation (filling them with bogus values and 
then quoting), we want to use all banks. Since we don't support hashing 
the data will all these different hashes at the same time, we would use 
the SHA 1 digest of the data for extension in the SHA256 bank etc.

>
> If so, could the code instead build the digest according to the
> hardware description instead of trying to reformat it after it is
> built?  Specifically, the only callers of tpm_extend() and
> tpm_log_event() produce a 'struct tcg_pcr_event2_sha1' with the digest
> in the simple hash format - could these locations create
> tcg_pcr_event2_sha1 in the desired hardware specified format initially
> and thus avoid needing to reformat that digest?

Yes, that would also be possible.

>
> [...]
>> +    if (dest && offset > destlen)
>> +        panic("buffer for tpml_digest_values is too small\n");
> panic() should be avoided.  On real hardware if the BIOS were to
> panic() it could effectively brick the machine.

Assuming I calculated the size correct, this error should never happen. 
So I have to remove this error report altogether or can I convert it 
into a print statement? I introduced this since I am not calculating the 
buffer size anymore...

    Stefan

>
> -Kevin
>