[SLOF] [PATCH 05/16] Extend internal firmware API

[Stefan Berger]

On 11/09/2015 03:56 AM, Nikunj A Dadhania wrote:
> Stefan Berger <stefanb at linux.vnet.ibm.com> writes:
>
>> Extend the internal API of the TPM firmware support with additional
>> functions for hashing data, extending the TPM's platform configuration
>> registers with a hash, and appending to the log that is recording
>> what was hashed.
>>
>> Signed-off-by: Stefan Berger <stefanb at linux.vnet.ibm.com>
>> ---
>>   lib/libtpm/tcgbios.c     | 276 +++++++++++++++++++++++++++++++++++++++++++++++
>>   lib/libtpm/tcgbios_int.h |   1 +
>>   2 files changed, 277 insertions(+)
>>
>> diff --git a/lib/libtpm/tcgbios.c b/lib/libtpm/tcgbios.c
>> index afbaede..072ed4d 100644
>> --- a/lib/libtpm/tcgbios.c
>> +++ b/lib/libtpm/tcgbios.c
>> @@ -24,6 +24,9 @@
>>   #include "tcgbios.h"
>>   #include "tcgbios_int.h"
>>   #include "stdio.h"
>> +#include "sha1.h"
>> +#include "stddef.h"
>> +#include "helpers.h"
>>
>>   #define DEBUG 0
>>   #define dprintf(_x ...) do { \
>> @@ -79,6 +82,8 @@ extern struct tpm_driver tpm_drivers[];
>>
>>   static void *log_base;
>>   static uint32_t log_area_size;
>> +/* next log entry goes here */
>> +static void *log_area_address_next;
>>
>>   /********************************************************
>>     Extensions for TCG-enabled BIOS
>> @@ -89,6 +94,7 @@ void tpm_set_log_parameters(void *addr, unsigned int size)
>>   	dprintf("Log is at 0x%llx; size is %u bytes\n",
>>   		(uint64_t)addr, size);
>>   	log_base = addr;
>> +	log_area_address_next = addr;
>>   	log_area_size = size;
>>   }
>>
>> @@ -399,3 +405,273 @@ err_exit:
>>   		return rc;
>>   	return TCGBIOS_COMMAND_ERROR;
>>   }
>> +
>> +static void set_log_area_address_next(void *next)
>> +{
>> +	log_area_address_next = next;
>> +}
> Where do you verify that next address is not more than log-area-size ?

This is verified when the blob is appended to the log. If it doesn't 
fit, this function will not be called.


>
>> +
>> +static void *get_log_area_address_next(void)
>> +{
>> +	return log_area_address_next;
>> +}
>> +
>> +static uint32_t tpm_sha1_calc(const uint8_t *data, uint32_t length,
>> +			      uint8_t *hash)
>> +{
>> +	uint32_t rc;
>> +	uint32_t returnCode;
>> +	struct tpm_res_sha1start start;
>> +	struct tpm_res_sha1complete complete;
>> +	uint32_t blocks = length / 64;
>> +	uint32_t rest = length & 0x3f;
> Better like this:
>
>          #define BLK_SIZE     64
>          
> 	uint32_t blocks = length / BLK_SIZE;
> 	uint32_t rest = length & (BLK_SIZE - 1);
>
>
>> +	uint32_t numbytes, numbytes_no;
> Bit confusing names :-)

_no stands for network (byte) order. Do you have a better suggestion for 
the name ?

>> +		return tpm_sha1_calc(data, length, hash);
>> +
>> +	return sha1(data, length, hash);
>> +}
>> +
>> +/*
>> + * Extend the ACPI log with the given entry by copying the
>> + * entry data into the log.
>> + *
>> + * @pcpes: Pointer to the structure to be copied into the log
>> + * @event: The event to be appended to 'pcpes'
>> + * @event_length: The length of the event
>> + *
>> + * Output:
>> + *  lower 16 bits of return code contain entry number
>> + *  if entry number is '0', then upper 16 bits contain error code.
>> + */
>> +static uint32_t tpm_extend_ofdt_log(struct pcpes *pcpes,
>> +				    const char *event, uint32_t event_length)
>> +{
>> +	uint32_t size;
>> +	uint32_t log_area_size = get_log_area_size();
>> +	uint8_t *log_area_start_address = get_log_base_ptr();
>> +	uint8_t *log_area_address_next = get_log_area_address_next();
>> +
>> +	dprintf("LASA_BASE = %p, LASA_NEXT = %p\n",
>> +		log_area_start_address, log_area_address_next);
>> +
>> +	if (!log_area_address_next || log_area_size == 0)
>> +		return TCGBIOS_LOGOVERFLOW;
>> +
>> +	size = offset_of(struct pcpes, event) + event_length;
>> +
>> +	if ((log_area_address_next + size - log_area_start_address) >
>> +	     log_area_size) {
>> +		dprintf("LOG OVERFLOW: size = %d\n", size);
>> +		return TCGBIOS_LOGOVERFLOW;
>> +	}
>> +
>> +	pcpes->eventdatasize = event_length;
>> +
>> +	memcpy(log_area_address_next, pcpes, offset_of(struct pcpes, event));
>> +	memcpy(log_area_address_next + offset_of(struct pcpes, event),
>> +	       event, event_length);
>> +
>> +	set_log_area_address_next(log_area_address_next + size);
>> +
>> +	return 0;
>> +}
>> +
>> +static uint32_t is_preboot_if_shutdown(void)
> Confusing function name. how about: is_interface_down(). Or just get rid
> of this helper, only being used at two places.
>
>> +{
>> +	return tpm_state.if_shutdown;
>> +}
>> +
>> +static uint32_t shutdown_preboot_interface(void)
>> +{
>> +	uint32_t rc = 0;
>> +
>> +	if (!is_preboot_if_shutdown()) {
>> +		tpm_state.if_shutdown = 1;
>> +	} else {
>> +		rc = TCGBIOS_INTERFACE_SHUTDOWN;
>> +	}
>> +
>> +	return rc;
>> +}
>> +
>> +static void tpm_shutdown(void)
>> +{
>> +	reset_ofdt_log();
>> +	shutdown_preboot_interface();
>> +}
>> +
>> +/*
>> + * Pass a TPM command through to the TPM
>> + *
>> + * @req: request buffer to send
>> + * @reqlen: request buffer length
>> + * @to_t: timeout type
>> + * @rsp: response buffer
>> + * @rsplen: size of response buffer in input
>> + *          on output the number of bytes used in the buffer
>> + *
>> + * Returns an error code or 0 for successful sending of command
>> + * and reception of response.
>> + */
>> +static bool pass_through_to_tpm(unsigned char *req,
> how about send_tpm_req() ?

The name of this function directly relates to the name of a API call of 
the firmware, which will later on invoke it. I would rather like to keep 
it....

>
>> +				uint32_t reqlen,
>> +				enum tpmDurationType to_t,
>> +				unsigned char *rsp,
>> +				uint32_t *rsplen)
>> +{
>> +	uint8_t locty = 0;
>> +	struct iovec iovec[2] = {{ 0 }};
>> +
>> +	if (!has_working_tpm())
>> +	       return TCGBIOS_FATAL_COM_ERROR;
>> +
>> +	iovec[0].data = req;
>> +	iovec[0].length = reqlen;
>> +
>> +	return transmit(locty, iovec, rsp, rsplen, to_t);
>> +}
>> +
>> +/*
>> + * Extend a PCR of the TPM with the given hash
>> + *
>> + * @hash: sha1 hash (20 bytes) to extend PCR with
>> + * @pcrindex: the PCR to extend [ 0..23 ]
>> + */
>> +static uint32_t tpm_extend(uint8_t *hash, uint32_t pcrindex)
>> +{
>> +	struct tpm_req_extend req = {
>> +		.tag	  = cpu_to_be16(TPM_TAG_RQU_CMD),
>> +		.totlen   = cpu_to_be32(sizeof(req)),
>> +		.ordinal  = cpu_to_be32(TPM_ORD_Extend),
>> +		.pcrindex = cpu_to_be32(pcrindex),
>> +	};
>> +	struct tpm_rsp_extend rsp;
>> +	uint32_t rsplen = sizeof(rsp);
>> +	uint32_t rc;
>> +
>> +	memcpy(req.digest, hash, sizeof(req.digest));
>> +
>> +	rc = pass_through_to_tpm((unsigned char *)&req, sizeof(req),
>> +				 TPM_DURATION_TYPE_SHORT,
>> +				 (unsigned char *)&rsp, &rsplen);
>> +	if (!rc) {
>> +		if (rsplen != sizeof(rsp)) {
>> +			dprintf("TPM_Extend response has unexpected size: %u\n",
>> +				rsplen);
>> +			rc = TCGBIOS_FATAL_COM_ERROR;
>> +		}
>> +	}
>> +
>> +	if (rc)
>> +		tpm_shutdown();
>> +
>> +	return rc;
>> +}
>> +
>> +/*
>> + * Hash then given input data and append the hash to the log
>             the
>
>> + *
>> + * @hashdata: the data to hash
>> + * @hashdatalen: the size of the data to hash
>> + * @pcpes: the 'pcpes' to append to the log; the hash will be written into this
>> + *         structure
>> + * @event: the event to append to the pcpes
>> + * @event_length: the lenth of the event array
>> + */
>> +static uint32_t hash_log_event(const void *hashdata,
>> +			       uint32_t hashdatalen,
>> +			       struct pcpes *pcpes,
>> +			       const char *event, uint32_t event_length)
>> +{
>> +	uint32_t rc;
>> +
>> +	if (is_preboot_if_shutdown())
>> +		return TCGBIOS_INTERFACE_SHUTDOWN;
>> +
>> +	if (pcpes->pcrindex >= 24)
> What does 24 mean? Comments?

The TPM has a maximum of 24 PCRs.

    Stefan