[SLOF] [PATCH 3/4] tcgbios: Use The proper sha function for each PCR bank

[Alexey Kardashevskiy]

On 30/06/2021 22:23, Stefan Berger wrote:
> 
> On 6/29/21 10:56 PM, Alexey Kardashevskiy wrote:
>>
>>
>> On 16/06/2021 00:41, Stefan Berger wrote:
>>> Instead of just using sha256 for all PCR banks (and truncating
>>> the value or zero-padding it) use the proper hash function for
>>> each one of the banks. For unimplemented hashes, fill the buffer
>>> with 0xff.
>>
>> Was it a bug that sha256 was always used?
>>
> Yes, it was a bug.

Put it in the commit log.


> 
> 
>>
>>>
>>> Signed-off-by: Stefan Berger <stefanb at linux.ibm.com>
>>> ---
>>>   lib/libtpm/tcgbios.c | 42 ++++++++++++++++++++++++++++++++----------
>>>   1 file changed, 32 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/lib/libtpm/tcgbios.c b/lib/libtpm/tcgbios.c
>>> index 91f2f46..af96c52 100644
>>> --- a/lib/libtpm/tcgbios.c
>>> +++ b/lib/libtpm/tcgbios.c
>>> @@ -127,28 +127,32 @@ static const struct hash_parameters {
>>>       uint8_t  hashalg_flag;
>>>       uint8_t  hash_buffersize;
>>>       const char *name;
>>> +    void (*hashfunc)(const uint8_t *data, uint32_t length, uint8_t 
>>> *hash);
>>>   } hash_parameters[] = {
>>>       {
>>>           .hashalg = TPM2_ALG_SHA1,
>>>           .hashalg_flag = TPM2_ALG_SHA1_FLAG,
>>>           .hash_buffersize = SHA1_BUFSIZE,
>>>           .name = "SHA1",
>>> +        .hashfunc = sha1,
>>>       }, {
>>>           .hashalg = TPM2_ALG_SHA256,
>>>           .hashalg_flag = TPM2_ALG_SHA256_FLAG,
>>>           .hash_buffersize = SHA256_BUFSIZE,
>>>           .name = "SHA256",
>>> +        .hashfunc = sha256,
>>>       }, {
>>>           .hashalg = TPM2_ALG_SHA384,
>>>           .hashalg_flag = TPM2_ALG_SHA384_FLAG,
>>>           .hash_buffersize = SHA384_BUFSIZE,
>>>           .name = "SHA384",
>>> -
>>> +        .hashfunc = sha384,
>>>       }, {
>>>           .hashalg = TPM2_ALG_SHA512,
>>>           .hashalg_flag = TPM2_ALG_SHA512_FLAG,
>>>           .hash_buffersize = SHA512_BUFSIZE,
>>>           .name = "SHA512",
>>> +        .hashfunc = sha512,
>>>       }, {
>>>           .hashalg = TPM2_ALG_SM3_256,
>>>           .hashalg_flag = TPM2_ALG_SM3_256_FLAG,
>>
>>
>>
>> Do not you want sha256/... for this and other guys after this one?
> 
> 
> These ones will return an array of 0xff for now. They will need their 
> own hash implementations in the future but for now only 
> SHA1,SHA256,SHA384 andSHA512 are enabled in the vTPM.

Than having hash_parameters structs without defined hashfunc is a bug 
too and we should just remove those until the proper support is added. 
Thanks,


> 
>    Stefan
> 
> 
> 
>>
>>
>>
>>> @@ -233,6 +237,25 @@ static const char * 
>>> tpm20_hashalg_flag_to_name(uint8_t hashalg_flag)
>>>       return NULL;
>>>   }
>>>   +static void tpm2_hash_data(uint16_t hashAlg,
>>> +                           const uint8_t *data, uint32_t data_len,
>>> +                           uint8_t *hash)
>>> +{
>>> +    unsigned i;
>>> +
>>> +    for (i = 0; i < ARRAY_SIZE(hash_parameters); i++) {
>>> +        if (hash_parameters[i].hashalg == hashAlg) {
>>> +            if (hash_parameters[i].hashfunc) {
>>> +                hash_parameters[i].hashfunc(data, data_len,
>>> +                                hash);
>>> +            } else {
>>> +                memset(hash, 0xff,
>>> +                       hash_parameters[i].hash_buffersize);
>>> +            }
>>> +        }
>>> +    }
>>> +}
>>> +
>>>   /*
>>>    * Build the TPM2 TPML_DIGEST_VALUES data structure from the given 
>>> hash.
>>>    * Follow the PCR bank configuration of the TPM and write the same 
>>> hash
>>> @@ -242,13 +265,15 @@ static const char * 
>>> tpm20_hashalg_flag_to_name(uint8_t hashalg_flag)
>>>    * hash when writing it in the area of the sha1 hash.
>>>    *
>>>    * le: the log entry to build the digest in
>>> - * sha1: the sha1 hash value to use
>>> + * hashdata: the data to hash
>>> + * hashdata_len: the length of the hashdata
>>>    * bigEndian: whether to build in big endian format for the TPM or log
>>>    *            little endian for the log (TPM 2.0)
>>>    *
>>>    * Returns the digest size; -1 on fatal error
>>>    */
>>> -static int tpm20_build_digest(struct tpm_log_entry *le, const 
>>> uint8_t *sha256,
>>> +static int tpm20_build_digest(struct tpm_log_entry *le,
>>> +                              const uint8_t *hashdata, uint32_t 
>>> hashdata_len,
>>>                     bool bigEndian)
>>>   {
>>>       struct tpms_pcr_selection *sel;
>>> @@ -297,9 +322,8 @@ static int tpm20_build_digest(struct 
>>> tpm_log_entry *le, const uint8_t *sha256,
>>>           else
>>>               v->hashAlg = cpu_to_le16(be16_to_cpu(sel->hashAlg));
>>>   -        memset(v->hash, 0, hsize);
>>> -        memcpy(v->hash, sha256,
>>> -               hsize < SHA256_BUFSIZE ? hsize : SHA256_BUFSIZE);
>>> +        tpm2_hash_data(be16_to_cpu(sel->hashAlg), hashdata, 
>>> hashdata_len,
>>> +                   v->hash);
>>>             dest += sizeof(*v) + hsize;
>>>           sel = nsel;
>>> @@ -865,7 +889,6 @@ static uint32_t 
>>> tpm_add_measurement_to_log(uint32_t pcrindex,
>>>                          const uint8_t *hashdata,
>>>                          uint32_t hashdatalen)
>>>   {
>>> -    uint8_t hash[SHA256_BUFSIZE];
>>>       struct tpm_log_entry le = {
>>>           .hdr.pcrindex = cpu_to_log32(pcrindex),
>>>           .hdr.eventtype = cpu_to_log32(eventtype),
>>> @@ -873,8 +896,7 @@ static uint32_t 
>>> tpm_add_measurement_to_log(uint32_t pcrindex,
>>>       int digest_len;
>>>       int ret;
>>>   -    sha256(hashdata, hashdatalen, hash);
>>> -    digest_len = tpm20_build_digest(&le, hash, true);
>>> +    digest_len = tpm20_build_digest(&le, hashdata, hashdatalen, true);
>>>       if (digest_len < 0)
>>>           return TCGBIOS_GENERAL_ERROR;
>>>       ret = tpm20_extend(&le, digest_len);
>>> @@ -882,7 +904,7 @@ static uint32_t 
>>> tpm_add_measurement_to_log(uint32_t pcrindex,
>>>           tpm_set_failure();
>>>           return TCGBIOS_COMMAND_ERROR;
>>>       }
>>> -    tpm20_build_digest(&le, hash, false);
>>> +    tpm20_build_digest(&le, hashdata, hashdatalen, false);
>>>       return tpm_log_event_long(&le.hdr, digest_len, info, infolen);
>>>   }
>>>
>>

-- 
Alexey