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

[Ilias Apalodimas]

On Sat, Nov 07, 2020 at 09:33:35PM +0100, Heinrich Schuchardt wrote:
> 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.
> 

Sure

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

Yes this was copied from this:
https://tpm2-tss.readthedocs.io/en/latest/

> > + */
> > +#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
> 

Ok will do

> > +
> > +/* 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.

I just extended what was already there. 
I agree we are better off changing it, not on this series though.

Regards
/Ilias
> 
> Best regards
> 
> Heinrich
> 
> >
> >  /* NV index attributes */
> >
>