[PATCH 2/3 v2] tpm: Add some headers from the spec

[Heinrich Schuchardt]

On 11/5/20 10:58 PM, Ilias Apalodimas wrote:
> A following patch introduces EFI_TCG2_PROTOCOL.
> Add the required TPMv2 headers to support that and remove the (now)
> redundant definitions from tpm2_tis_sandbox
>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> ---
>  include/tpm-v2.h | 69 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 69 insertions(+)
>
> diff --git a/include/tpm-v2.h b/include/tpm-v2.h
> index f6c045d35480..b62f2c5b0fb8 100644
> --- a/include/tpm-v2.h
> +++ b/include/tpm-v2.h
> @@ -11,6 +11,73 @@
>
>  #define TPM2_DIGEST_LEN		32

Miquel, the constant name and its usage seems not to reflect the TCG spec:

The spec has the following alternative digest length constants:

#define TPM2_SHA_DIGEST_SIZE       20
#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

Can't we use the same names as in the spec? Why did you assume in your
code that SHA256 is the only digest being used?

>
> +#define TPM2_MAX_PCRS 32
> +#define TPM2_PCR_SELECT_MAX ((TPM2_MAX_PCRS + 7) / 8)
> +#define TPM2_MAX_CAP_BUFFER 1024
> +#define TPM2_MAX_TPM_PROPERTIES   ((TPM2_MAX_CAP_BUFFER - sizeof(u32) /* TPM2_CAP */ - \
> +				   sizeof(u32)) / sizeof(struct tpms_tagged_property))
> +
> +/*
> + *  We deviate from this draft of the specification by increasing the value of TPM2_NUM_PCR_BANKS
> + *  from 3 to 16 to ensure compatibility with TPM2 implementations that have enabled a larger than
> + *  typical number of PCR banks. This larger value for TPM2_NUM_PCR_BANKS is expected to be included
> + *  in a future revision of the specification.

Ilias, please, restrict your lines to 80 characters where possible.

Do you have a reference for the usage of that larger number on current
hardware? We should make the deviation from the standard easily verifiable.

> + */
> +#define TPM2_NUM_PCR_BANKS 16
> +
> +/* Definition of (UINT32) TPM2_CAP Constants */
> +#define TPM2_CAP_PCRS 0x00000005U
> +#define TPM2_CAP_TPM_PROPERTIES 0x00000006U
> +
> +/* Definition of (UINT32) TPM2_PT Constants */
> +#define PT_GROUP                   (u32)(0x00000100)
> +#define PT_FIXED                   (u32)(PT_GROUP * 1)
> +#define TPM2_PT_MANUFACTURER        (u32)(PT_FIXED + 5)
> +#define TPM2_PT_PCR_COUNT           (u32)(PT_FIXED + 18)
> +#define TPM2_PT_MAX_COMMAND_SIZE    (u32)(PT_FIXED + 30)
> +#define TPM2_PT_MAX_RESPONSE_SIZE   (u32)(PT_FIXED + 31)

All these definitions are all copied from the "TCG TSS2.0 Overview and
Common Structures Specification". I am missing a reference to the
copyright notice of the spec. I think the best thing to do would be
placing the TCG copyrighted code into a separate include that is
included in tpm_v2.h. Please, check with Tom if the license contradicts
GPL. Especially the following sentence seems problematic:

"THE COPYRIGHT LICENSES SET FORTH ABOVE DO NOT REPRESENT ANY FORM OF
LICENSE OR WAIVER, EXPRESS OR IMPLIED, BY ESTOPPEL OR OTHERWISE, WITH
RESPECT TO PATENT RIGHTS HELD BY TCG MEMBERS (OR OTHER THIRD PARTIES)
THAT MAY BE NECESSARY TO IMPLEMENT THIS SPECIFICATION OR OTHERWISE."

Cf. https://fedoraproject.org/wiki/Licensing/TCGL

> +
> +/* TPMS_TAGGED_PROPERTY Structure */
> +struct tpms_tagged_property {
> +	u32 property;
> +	u32 value;
> +} __packed;
> +
> +/* TPMS_PCR_SELECTION Structure */
> +struct tpms_pcr_selection {
> +	u16 hash;
(This is a TPM2_ALG_ID.)
> +	u8 size_of_select;
> +	u8 pcr_select[TPM2_PCR_SELECT_MAX];
> +} __packed;
> +
> +/* TPML_PCR_SELECTION Structure */
> +struct tpml_pcr_selection {
> +	u32 count;
> +	struct tpms_pcr_selection selection[TPM2_NUM_PCR_BANKS];
> +} __packed;
> +
> +/* TPML_TAGGED_TPM_PROPERTY Structure */
> +struct tpml_tagged_tpm_property {
> +	u32 count;
> +	struct tpms_tagged_property tpm_property[TPM2_MAX_TPM_PROPERTIES];
> +} __packed;
> +
> +/* TPMU_CAPABILITIES Union */
> +union tpmu_capabilities {
> +	/*
> +	 * Non exhaustive. Only added the structs needed for our
> +	 * current code
> +	 */
> +	struct tpml_pcr_selection assigned_pcr;
> +	struct tpml_tagged_tpm_property tpm_properties;
> +} __packed;
> +
> +/* TPMS_CAPABILITY_DATA Structure */
> +struct tpms_capability_data {
> +	u32 capability;
> +	union tpmu_capabilities data;
> +} __packed;
> +
>  /**
>   * TPM2 Structure Tags for command/response buffers.
>   *
> @@ -123,11 +190,13 @@ enum tpm2_return_codes {
>   * TPM2 algorithms.
>   */
>  enum tpm2_algorithms {
> +	TPM2_ALG_SHA1		= 0x04,
>  	TPM2_ALG_XOR		= 0x0A,
>  	TPM2_ALG_SHA256		= 0x0B,
>  	TPM2_ALG_SHA384		= 0x0C,
>  	TPM2_ALG_SHA512		= 0x0D,
>  	TPM2_ALG_NULL		= 0x10,
> +	TPM2_ALG_SM3_256	= 0x12,
>  };

In the spec these values are not an enum (i.e. 32 bit values if you do
not use short enums) but explicitly u16/TPM2_ALG_ID. But probably that
does not make a difference.

Best regards

Heinrich

>
>  /* NV index attributes */
>