tpm2-software / tpm2-tss

OSS implementation of the TCG TPM2 Software Stack (TSS2)
https://tpm2-software.github.io
BSD 2-Clause "Simplified" License
755 stars 366 forks source link

Mismatch in implementation TPM2B_DATA structure vs. documentation/specification #2888

Open dlegalov-fortanix opened 3 months ago

dlegalov-fortanix commented 3 months ago

There is a mismatch in the way the TPM2B_DATA is implemented in this library compared to its definition in the specification.

Implementation

In the source code, the structure is as follows:

/* Definition of TPM2B_DATA Structure */
typedef struct TPM2B_DATA TPM2B_DATA;
struct TPM2B_DATA {
    UINT16 size;
    BYTE buffer[sizeof(TPMU_HA)];
};

(https://github.com/tpm2-software/tpm2-tss/blob/master/include/tss2/tss2_tpm2_types.h#L981)

Specification

In the documentation linked from the GitHub, (https://trustedcomputinggroup.org/wp-content/uploads/TCG_TSS_Overview_Common_Structures_v0.9_r03_published.pdf) the structure is defined as

/* Table 74 - Definition of TPM2B_DATA Structure */
typedef struct {
 UINT16 size;
 BYTE buffer[sizeof(TPMT_HA)];
} TPM2B_DATA;

The specification documents from TCG, (https://trustedcomputinggroup.org/resource/tpm-library-specification/) provide the following definitions:

https://trustedcomputinggroup.org/wp-content/uploads/TPM-2.0-1.83-Part-2-Structures.pdf image

From the document provided at http://standards.iso.org/ittf/PubliclyAvailableStandards/c066511_ISO_IEC_11889-2_2015.zip the definition is as such: image

Discrepancy

The issue is that the implementation uses sizeof(TPMU_HA), as opposed to sizeof(TPMT_HA). This is a difference of 2 bytes, with the implementation being smaller.

One concern with this difference is that a clean-room implementation of the system that only uses the documentation will have a different size of any structure that includes TPM2B_DATA, such as TPMT_ATTEST. Proper marshaling checks should prevent serious issues, but it is something that should be noted, and potentially resolved in the future.

AndreasFuchsTPM commented 3 months ago

This is something we will fix if we have to change ABI anyways. But for now we will stick with the wrong size because it does not warrant rolling an ABI change just for this. It is only used for the externalData nonce in the Esys_Quote and Esys_Certify calls where the nonce is now restricted to 64 bytes. "Older" TPMs (with max SHA384) only support 50 bytes anyways. In future, TPMs supporting 66 bytes (via SHA512) will have to sacrifice to 64 bytes in applications instead. I will make an addition to the README.