diff mbox series

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

Message ID 20201127162932.1965323-2-ilias.apalodimas@linaro.org
State New
Headers show
Series extend EFI_TCG2_PROTOCOL support | expand

Commit Message

Ilias Apalodimas Nov. 27, 2020, 4:29 p.m. UTC
A following patch introduces support for the EFI_TCG2_PROTOCOL
evenlog management.
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(+)

-- 
2.29.2

Comments

Heinrich Schuchardt Nov. 29, 2020, 5:28 a.m. UTC | #1
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?

>

> +#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.

> +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?

Otherwise looks ok to me.

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.

>    *

>
Ilias Apalodimas Nov. 29, 2020, 1:22 p.m. UTC | #2
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.

> >    *

> > 

>
diff mbox series

Patch

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
 
+#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 */
+struct tcg_pcr_event {
+	u32 pcr_index;
+	u32 event_type;
+	struct tpm_digest digest;
+	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.
  *