[U-Boot] [PATCH v3 12/25] tpm: add TPM2_PCR_Read command support

[Miquel Raynal]

Hi Simon,

On Wed, 2 May 2018 20:32:10 -0600, Simon Glass <sjg at chromium.org> wrote:

> Hi Miquel,
> 
> On 2 May 2018 at 02:59, Miquel Raynal <miquel.raynal at bootlin.com> wrote:
> > Add support for the TPM2_PCR_Read command.
> >
> > Change the command file and the help accordingly.
> >
> > Signed-off-by: Miquel Raynal <miquel.raynal at bootlin.com>
> > ---
> >  cmd/tpm-v2.c     | 27 +++++++++++++++++++++++++++
> >  include/tpm-v2.h | 11 +++++++++++
> >  lib/tpm-v2.c     | 43 +++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 81 insertions(+)
> >  
> 
> Reviewed-by: Simon Glass <sjg at chromium.org>
> 
> nits below
> 
> > diff --git a/cmd/tpm-v2.c b/cmd/tpm-v2.c
> > index 6e19adbfe6..a61d751b4a 100644
> > --- a/cmd/tpm-v2.c
> > +++ b/cmd/tpm-v2.c
> > @@ -86,6 +86,28 @@ static int do_tpm2_pcr_extend(cmd_tbl_t *cmdtp, int flag, int argc,
> >         return report_return_code(tpm2_pcr_extend(index, digest));
> >  }
> >
> > +static int do_tpm_pcr_read(cmd_tbl_t *cmdtp, int flag, int argc,
> > +                          char * const argv[])
> > +{
> > +       u32 index, rc;
> > +       unsigned int updates;
> > +       void *data;
> > +
> > +       if (argc != 3)
> > +               return CMD_RET_USAGE;
> > +
> > +       index = simple_strtoul(argv[1], NULL, 0);
> > +       data = (void *)simple_strtoul(argv[2], NULL, 0);  
> 
> data = map_sysmem(simple...(), 0);
> 
> so that it works on sandbox.

I replaced all the similar lines in my code. Thank you very much for
this hint.

> 
> [...]
> 
> > +/**
> > + * Issue a TPM2_PCR_Read command.
> > + *
> > + * @param index                Index of the PCR
> > + * @param data         Output buffer for contents of the named PCR
> > + * @param updates      Optional out parameter: number of updates for this PCR
> > + *
> > + * @return return code of the operation  
> 
> For new code we should use
> 
> @index: Index of the PCR
> @data: Output ...
> @updates: Optional ....
> @return ..

Sure. I changed that also everywhere in new code.

> 
> 
> > + */
> > +u32 tpm2_pcr_read(u32 index, void *data, unsigned int *updates);
> > +
> >  #endif /* __TPM_V2_H */
> > diff --git a/lib/tpm-v2.c b/lib/tpm-v2.c
> > index 2696f8145d..d557b08f8b 100644
> > --- a/lib/tpm-v2.c
> > +++ b/lib/tpm-v2.c
> > @@ -115,3 +115,46 @@ u32 tpm2_pcr_extend(u32 index, const uint8_t *digest)
> >
> >         return tpm_sendrecv_command(command_v2, NULL, NULL);
> >  }
> > +
> > +u32 tpm2_pcr_read(u32 index, void *data, unsigned int *updates)
> > +{
> > +       u8 command_v2[COMMAND_BUFFER_SIZE] = {
> > +               tpm_u16(TPM2_ST_NO_SESSIONS),   /* TAG */
> > +               tpm_u32(20),                    /* Length */
> > +               tpm_u32(TPM2_CC_PCR_READ),      /* Command code */
> > +
> > +               /* TPML_PCR_SELECTION */
> > +               tpm_u32(1),                     /* Number of selections */
> > +               tpm_u16(TPM2_ALG_SHA256),       /* Algorithm of the hash */
> > +               3,                              /* Array size for selection */
> > +               /* bitmap(index)                Selected PCR bitmap */
> > +       };
> > +       size_t response_len = COMMAND_BUFFER_SIZE;
> > +       u8 response[COMMAND_BUFFER_SIZE];
> > +       unsigned int counter = 0;
> > +       u8 pcr_sel[3] = {};
> > +       int ret;
> > +
> > +       if (index >= 24)  
> 
> What is 24?

This value was the number of PCRs. It was wrong as the specification
does not limit this number to be 24. I changed the code so this check
is done somewhere else dynamically depending on the actual limitations.

I also simplified a bit the following section.

> 
> > +               return TPM_LIB_ERROR;
> > +
> > +       pcr_sel[index / 8] = BIT(index % 8);
> > +       if (pack_byte_string(command_v2, COMMAND_BUFFER_SIZE, "bbb",
> > +                            17, pcr_sel[0], 18, pcr_sel[1], 19, pcr_sel[2]))
> > +               return TPM_LIB_ERROR;
> > +
> > +       ret = tpm_sendrecv_command(command_v2, response, &response_len);
> > +       if (ret)
> > +               return ret;
> > +
> > +       if (unpack_byte_string(response, response_len, "ds",
> > +                              10, &counter,
> > +                              response_len - TPM2_DIGEST_LEN, data,
> > +                              TPM2_DIGEST_LEN))
> > +               return TPM_LIB_ERROR;
> > +
> > +       if (updates)
> > +               *updates = counter;
> > +
> > +       return 0;
> > +}
> > --
> > 2.14.1
> >  
> 
> Regards,
> Simon

Thanks,
Miquèl

-- 
Miquel Raynal, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com