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

[Stefan Berger]

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.


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

   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);
>>   }
>>
>