[PATCH 1/3] tpm: Add tpm2 headers for TCG2 eventlog support

[Ilias Apalodimas <ilias.apalodimas@linaro.org>]

Hi Heinrich,

On Sun, Nov 29, 2020 at 06:28:39AM +0100, Heinrich Schuchardt wrote:
> On 11/27/20 5:29 PM, Ilias Apalodimas wrote:
> > A following patch introduces support for the EFI_TCG2_PROTOCOL
> > evenlog management.
> 
> %s/evenlog/eventlog/
> 
> > Introduce the necessary tpm related headers
> > 
> > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > ---
> >   include/tpm-v2.h | 59 ++++++++++++++++++++++++++++++++++++++++++++++++
> >   1 file changed, 59 insertions(+)
> > 
> > diff --git a/include/tpm-v2.h b/include/tpm-v2.h
> > index d8c9feda28dc..9637f9be998e 100644
> > --- a/include/tpm-v2.h
> > +++ b/include/tpm-v2.h
> > @@ -18,6 +18,12 @@
> > 
> >   #define TPM2_DIGEST_LEN		32
> 
> Shouldn't TPM2_DIGEST_LEN be renamed to TPM2_SHA256_DIGEST_SIZE in all
> of our code?

Ideally yes. The current tpm2 pre-existing code (apart from tpm2_pcr_extend()
which I changed) uses a hardware SHA256 algorithm.
Should we do it in this patchset though?

> 
> > 
> > +#define TPM2_SHA1_DIGEST_SIZE 20
> > +#define TPM2_SHA256_DIGEST_SIZE	32
> > +#define TPM2_SHA384_DIGEST_SIZE	48
> > +#define TPM2_SHA512_DIGEST_SIZE	64
> > +#define TPM2_SM3_256_DIGEST_SIZE 32
> > +
> >   #define TPM2_MAX_PCRS 32
> >   #define TPM2_PCR_SELECT_MAX ((TPM2_MAX_PCRS + 7) / 8)
> >   #define TPM2_MAX_CAP_BUFFER 1024
> > @@ -45,6 +51,15 @@
> >   #define TPM2_PT_MAX_COMMAND_SIZE	(u32)(TPM2_PT_FIXED + 30)
> >   #define TPM2_PT_MAX_RESPONSE_SIZE	(u32)(TPM2_PT_FIXED + 31)
> > 
> > +/* event types */
> > +#define EV_POST_CODE		((u32)0x00000001)
> > +#define EV_NO_ACTION		((u32)0x00000003)
> > +#define EV_SEPARATOR		((u32)0x00000004)
> > +#define EV_S_CRTM_CONTENTS	((u32)0x00000007)
> > +#define EV_S_CRTM_VERSION	((u32)0x00000008)
> > +#define EV_CPU_MICROCODE	((u32)0x00000009)
> > +#define EV_TABLE_OF_DEVICES	((u32)0x0000000B)
> > +
> >   /* TPMS_TAGGED_PROPERTY Structure */
> >   struct tpms_tagged_property {
> >   	u32 property;
> > @@ -86,6 +101,50 @@ struct tpms_capability_data {
> >   	union tpmu_capabilities data;
> >   } __packed;
> > 
> > +/* defined as TPM_SHA1_160_HASH_LEN in spec */
> > +struct tpm_digest {
> > +	u8 digest[TPM2_SHA1_DIGEST_SIZE];
> > +} __packed;
> > +
> > +/* SHA1 Event Log Entry Format */
> 
> Please, use Sphinx style comments for structures. This allows us to add
> them to the HTML documentation. See
> 
> https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#structure-union-and-enumeration-documentation
> 
> I would appreciate member descriptions, too.
> 

Ok I assumed refering to the spec would be fine.
I'll add description on v2

> > +struct tcg_pcr_event {
> > +	u32 pcr_index;
> > +	u32 event_type;
> > +	struct tpm_digest digest;
> 
> struct tpm_digest is only used here in your patch series.
> 
> The standard has
> 
>     typedef UINT8 TCG_DIGEST[TPM2_SHA1_DIGEST_SIZE];
> 
> Can't we simply write
> 
> 	u8 digest[20];
> 
> here and get rid of struct tpm_digest?

Yea we can. since it's a complex spec though I am trying to adhere
to it as much as possible to make review and future extentions easier.
I'd prefer keeping this as is tbh.

> 
> Otherwise looks ok to me.
> 

Thanks for the review!. I'll wait a few more days, fix your remarks and post a v2

Cheers
/Ilias
> Best regards
> 
> Heinrich
> 
> > +	u32 event_size;
> > +	u8 event[];
> > +} __packed;
> > +
> > +/* Definition of TPMU_HA Union */
> > +union tmpu_ha {
> > +	u8 sha1[TPM2_SHA1_DIGEST_SIZE];
> > +	u8 sha256[TPM2_SHA256_DIGEST_SIZE];
> > +	u8 sm3_256[TPM2_SM3_256_DIGEST_SIZE];
> > +	u8 sha384[TPM2_SHA384_DIGEST_SIZE];
> > +	u8 sha512[TPM2_SHA512_DIGEST_SIZE];
> > +} __packed;
> > +
> > +/* Definition of TPMT_HA Structure */
> > +struct tpmt_ha {
> > +	u16 hash_alg;
> > +	union tmpu_ha digest;
> > +} __packed;
> > +
> > +/* Definition of TPML_DIGEST_VALUES Structure */
> > +struct tpml_digest_values {
> > +	u32 count;
> > +	struct tpmt_ha digests[TPM2_NUM_PCR_BANKS];
> > +} __packed;
> > +
> > +/* Crypto Agile Log Entry Format */
> > +struct tcg_pcr_event2 {
> > +	u32 pcr_index;
> > +	u32 event_type;
> > +	struct tpml_digest_values digests;
> > +	u32 event_size;
> > +	u8 event[];
> > +} __packed;
> > +
> >   /**
> >    * TPM2 Structure Tags for command/response buffers.
> >    *
> > 
>