[SeaBIOS] [PATCH v3 4/7] tpm: Extend tpm20_extend to support extending to multiple PCR banks

[Kevin O'Connor]

On Tue, Aug 09, 2016 at 10:23:36PM -0400, Stefan Berger wrote:
> On 08/09/2016 01:36 PM, Kevin O'Connor wrote:
> > On Fri, Aug 05, 2016 at 11:07:11AM -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.
> > [...]
> > > @@ -573,7 +700,16 @@ tpm_add_measurement_to_log(u32 pcrindex, u32 event_type,
> > >           }
> > >       };
> > >       sha1(hashdata, hashdata_length, entry.digest.sha1);
> > > -    int ret = tpm_extend(entry.pcrindex, entry.digest.sha1);
> > > +
> > > +    u8 buffer[MAX_TPML_DIGEST_VALUES_SIZE];
> > > +
> > > +    int tdv_len = tpm_write_tpml_digest_values(buffer, sizeof(buffer),
> > > +        entry.digest.sha1, TPM2_ALG_SHA1);
> > > +    if (tdv_len < 0)
> > > +        return;
> > > +
> > > +    struct tpml_digest_values *tdv = (struct tpml_digest_values *)buffer;
> > > +    int ret = tpm_extend(pcrindex, tdv, tdv_len);
> > >       if (ret) {
> > >           tpm_set_failure();
> > >           return;
> > If the code introduced a new struct with the maximum sized log entry
> > instead of declaring buffers of size MAX_TPML_DIGEST_VALUES_SIZE I
> > think the code would be a little simpler.  That new struct could then
> > be used for both tpm_extend() and tpm_log_event() making patches 5-7
> > simpler.
> > 
> > See https://github.com/KevinOConnor/seabios/tree/testing for what I
> > was thinking.
> 
> So I tested this and your code works just as well.
> These data structures are a bit tricky and it takes a while to see how for
> example the tpm_log_header is used for TPM 1.2 and TPM 2. I left a comment
> in the patch on github. Maybe you could leave a comment in the
> tpm_log_header datastructure stating that digest holds the SHA1 hash in TPM
> 1.2 case and a tpm2_digest_values struct in TPM 2 case. Otherwise I think
> you can merge this in.

How about the update below (which I'll squash into the appropriate
patches).

-Kevin


--- a/src/std/tcg.h
+++ b/src/std/tcg.h
@@ -518,6 +518,11 @@ struct tpm2_digest_values {
     struct tpm2_digest_value digest[0];
 } PACKED;
 
+// Each entry in the TPM log contains: a tpm_log_header, a variable
+// length digest, a tpm_log_trailer, and a variable length event.  On
+// TPM1.2 the digest is a SHA1 hash; on TPM2.0 the digest contains a
+// tpm2_digest_values struct followed by a variable number of
+// tpm2_digest_value structs.
 struct tpm_log_header {
     u32 pcrindex;
     u32 eventtype;
diff --git a/src/tcgbios.c b/src/tcgbios.c
index a3074e2..7f045e3 100644
--- a/src/tcgbios.c
+++ b/src/tcgbios.c
@@ -136,6 +136,11 @@ tpm_tcpa_probe(void)
 /*
  * Extend the ACPI log with the given entry by copying the
  * entry data into the log.
+ * Input
+ *  entry : The header data to use (including the variable length digest)
+ *  digest_len : Length of the digest in 'entry'
+ *  event : Pointer to the event body to be copied into the log
+ *  event_len : Length of 'event'
  *
  * Output:
  *  Returns an error code in case of faiure, 0 in case of success
@@ -197,7 +202,10 @@ tpm_can_show_menu(void)
     return 0;
 }
 
-// Maximum supported log entry header (header + digest)
+// A 'struct tpm_log_entry' is a local data structure containing a
+// 'tpm_log_header' followed by space for the maximum supported
+// digest.  (The digest is a sha1 hash on tpm1.2 or a series of
+// tpm2_digest_value structs on tpm2.0)
 struct tpm_log_entry {
     struct tpm_log_header hdr;
     u8 pad[sizeof(struct tpm2_digest_values)
@@ -360,6 +368,7 @@ tpm20_build_digest(struct tpm_log_entry *le, const u8 *sha1)
 static int
 tpm12_build_digest(struct tpm_log_entry *le, const u8 *sha1)
 {
+    // On TPM 1.2 the digest contains just the SHA1 hash
     memcpy(le->hdr.digest, sha1, SHA1_BUFSIZE);
     return SHA1_BUFSIZE;
 }